magibney commented on PR #1895: URL: https://github.com/apache/solr/pull/1895#issuecomment-2153378871
Thanks, yes I will definitely keep an eye on the builds. I'll wait a few days -- @markrmiller if you're able to weigh in that would be much appreciated! I do have a specific question about [this](https://github.com/apache/solr/blob/11141c0c6a788213e15a9eff1b5fe7e2ea4870ad/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java#L196): ```java UpdateLog existingLog = updateHandler.getUpdateLog(); if (this.ulog != null && this.ulog == existingLog) { // If we are reusing the existing update log, inform the log that its update handler has // changed. We do this as late as possible. this.ulog.init(this, core); } ``` ... specifically, why do we need to do this "as late as possible"? afaict the only thing the UpdateHandler is used for is to update the UpdateLog's ref to the UpdateHandler -- no fields, etc. consulted. This would be far simpler if we _always_ called `ulog.init(this, core)` from the super ctor (`UpdateHandler`), as long as `ulog != null`. I'm also particularly interested in feedback on [this TODO](https://github.com/apache/solr/blob/962b99f430c56f7387fbf233950fbc12946aaa70/solr/core/src/java/org/apache/solr/update/UpdateLog.java#L431-L436): ```java // TODO: why do we need fancy concurrency constructs here? I don't see where this would // actually be called concurrently. Could `TestHdfsUpdateLog.testFSThreadSafety()` // (introduced by SOLR-7113) in fact be the only place that requires this? Or perhaps we // need the `init(UpdateHandler, SolrCore)` method in its entirety (or some large portion of // it) to synchronize on `this`, to ensure visibility of non-final fields that are // initialized? ``` -- 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
