to be clear because I don't want to come off as antagonizing... the author of the PR could be anyone. I think if we forget who and focus on what we can all row together.
I'm writing the group as a whole because I do want to contest this sort of way of doing things in the absence of good reason. I've modified my initial email below to be a bit more focused. Thanks all, Marcus On Tue, Aug 11, 2020 at 11:17 PM 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, some writes that they 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 > the experts, 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. 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. Does the author u 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 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 > > -- Marcus Eagan
