I don’t have time to write a more comprehensive email with links and references but the basic outline here is that Mark did some (read: a ton) work on a private branch that he later shared with his coworkers. Pieces of that branch filtered out to Apache.
The intent behind this annotations was to start labeling things so that we can reason about our concurrency better, since right now we likely have bugs relating to misuse of concurrent classes and shared state. Solr is a complex beast that is very hard to understand, so the thought was that we could start chipping away at it. Mike On Wed, Aug 12, 2020 at 1:18 AM Marcus Eagan <[email protected]> wrote: > Hi Community, > > As I have taken it upon myself to slowly document opportunities for > improving the overall quality and maintainability of the code base, I was > looking into my notes on test failures. > > As you all know many test failures in distributed systems repos sometimes > can be traced back to race conditions or trying to simulate thread safety > with waits. So I went digging into any work in this area. > > I stumbled across a peculiar contribution: SOLR-13998: Add thread safety > annotations to classes (#1053) > <https://github.com/apache/lucene-solr/pull/1057>. In the title of the > PR, @Anshum Gupta <[email protected]> writes that he adds thread > safety annotation to classes. > > This is misleading and confusing to me for multiple reasons: > > 1. I was thrown off a little bit by the title. While I tend to defer to > Anshum the expert, and I think it might be helpful to have thread safety > annotations for the compiler in many cases, there were literally no > annotations added to any classes in this PR. H the changes notes from the > PR say: > >> SOLR-13998: Add thread safety annotations to Solr. This only introduces >> the annotations and doesn't add these to existing classes. (Anshum >> Gupta, Mark Miller) >> existing classes. > > > Since this is true, I think the title should change. What was Mark's > involvement? Is there a commit/comment missing? > > 2. What is the value in adding this SingleThreaded annotation 8 months > ago, yet not implementing it once since? In fact, it has only been used > once, in one file by Dat, which is great. The only place I found it was > here: https://github.com/apache/lucene-solr/pull/1470 from last month's > shard handler improvements from Dat. I'm mostly concerned about how the > project operates with regard to this question. > > 3. As a general engagement model that could help the project, I think that > if someone is going to seemingly add something to Solr, they should > implement it in a few places, or point the rest of the community at places > where they think it could help the project. Getting in the habit of adding > a class, and then not using it anywhere should be something that the > committers and contributors on the project reconsider. Otherwise, the > codebase will be sprawling. It is sprawling in some areas. More on that > later. There are other much older manifestations of this behavior, so I am > asking the community, in particular the contributor of this recently added > but scantly implemented class, to take a more active role in spearheading > the addition of the SolrSingleThreaded annotations where/if they can add > value. > > 4. Ideas I have around this: > a) maybe you could share why you added this code in the first place. > b) where and why you or the community see we should be annotating more in > this manner. > c) re-examine your own contribution and ask yourself "should I have > contributed this code without more discussion about how it would be > implemented in the project?" I stumbled upon this looking for what the PR > says it is, but in reality there's not much there. I understand that, and I > really like micro-PRs. They're easier to review. But I think we should take > this opportunity to build on that work, or get rid of it and other PRs like > it. > > 5. Anshum, did you have some other objectives or aims back in December > when you added the SolrSingleThreaded annotation? Does anyone else have any > objectives around supporting this effort? Can I and others be of assistance > in helping you get to where you intended to go with the PR? I really want > to be helpful. > > 6. Most of all, maybe Anshum or someone here can actually explain to me > what SolrSingleThreaded actually does that is needed? I have thoughts and > have used annotations like this before with Spring, but in my very limited > knowledge about what this PR was intended to accomplish, this > implementation does nothing. > > Please advise, > > Marcus > > > > > -- > Marcus Eagan > >
