That is a great thought and I agree with it. On Wed, Aug 12, 2020 at 5:55 AM Mike Drob <[email protected]> wrote:
> 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 >> >> -- Marcus Eagan
