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
