kotman12 commented on PR #4577: URL: https://github.com/apache/solr/pull/4577#issuecomment-4938295304
Regarding this thread @linxiaokun528 @ercsonusharma >FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change. The bug is purely in the detection logic which is fixable while keeping the interfaces intact. ... > While OnReconnect is triggered, we use AtomicBoolean.compareAndSet to set it back to false. But in this way, the two methods are tightly coupled. If you look at when `OnReconnect` is [triggered in 9.X](https://github.com/apache/solr/blob/releases/solr/9.10.1/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java#L156-L200) it is effectively an `OnExpired` hook. I think it always has been until the curator migration so your implementation of on "expired" is compatible with the original `OnReconnect`. The downside is that to not break the API you have to continue this misleading naming convention. One could easily argue this misleading naming convention is what caused this bug in the first place (confusion around what is a reconnection vs what is a session expiry + reconnection). So to echo @dsmiley's point I think from a purely _technical_ standpoint it is better to get rid of `OnReconnect`. Now whether to replace it with a `OnSessionExpiryReconnect` abstract class or use this new enum with anonymous class is another matter. I personally don't have strong feelings either way. Regarding `OnDisconnect` specifically this is a brand new addition in 10.X so IMO it is ok to drop since it has never really worked (I don't consider 10.0 to really be a working/stable release yet given problems like the one addressed by this PR). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
