magibney commented on PR #1895:
URL: https://github.com/apache/solr/pull/1895#issuecomment-2155373058

   For the record, considering the above question about concurrency constructs:
   
   I've dug into [SOLR-7113](https://issues.apache.org/jira/browse/SOLR-7113), 
and I'm increasingly convinced that although it's technically correct that 
`UpdateLog.init(UpdateHandler, SolrCore)` is not thread safe, the issue title 
(filed with initial report) is misleading in calling attention to "thread 
safety", and the issue fix (in the form of making a portion of the method 
synchronized on `fsLock` object) is misleading, because afaict 
`UpdateLog.init(UpdateHandler, SolrCore)` is never called concurrently in 
application code, nor should it be.
   
   The issue with SOLR-7113 was not one of thread safety, so much as one of 
leaking resources and closing them across different scopes. This issue was 
effectively fixed by disallowing modification of tlog location across core 
reloads.
   
   My inclination atm is to leave the AtomicBoolean in place -- we need 
something to shortcircuit repeated init in application code (which does happen, 
albeit not in a way that needs to be thread safe), and leaving AtomicBoolean is 
thread-safe against the test that was written for SOLR-7113; so AtomicBoolean 
gets the job done for both, even if it's not strictly necessary as far as 
application code goes.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to