nick-boss-tech commented on PR #5301: URL: https://github.com/apache/solr/pull/5301#issuecomment-6758903946
🤖 *AI text below* 🤖 *(reviewed by Nick Shanin)* Thanks for explaining why you removed the expiry guard. I agree that an expiry still needs to trigger recovery if it belongs to the session that needs replacing. I found a case where the queued expiry belongs to an older replacement, but recovery has already installed a healthy new session. Processing that expiry starts recovery again and closes the healthy session. The [test](https://github.com/nick-boss-tech/solr/compare/684354d21476e44458e45c83a810dd25d16ba284...b5477834e588888c542409bebbe2d1b76bbade27) reproduces that ordering with real ZooKeeper sessions, using a log appender to hold a callback. On Linux with seed `5301C0DE`, it fails on your current production code at `stale expiry disconnected the recovered session`. With the same test and my earlier sequence-guarded `ConnectionManager.java` substituted as a control, it passes and retains the recovered session. Tidy and `check -x test` passed on the test branch. This is a controlled ordering; I don't know how often it occurs in practice. My earlier sequence guard passes this case as a control, but I haven't established that it safely handles every expiry ordering you were concerned about. I think we should distinguish an expiry that still needs recovery from one whose session has already been replaced. The branch adds only the test and is based on your current head. The test is expected to fail until the stale-expiry behavior is addressed. To try it: ```sh git switch SOLR-18623-connection-manager-event-ordering git fetch https://github.com/nick-boss-tech/solr.git pr-5301-stale-expiry-regression git merge --ff-only b5477834e588888c542409bebbe2d1b76bbade27 ./gradlew :solr:core:cleanTest :solr:core:test --tests org.apache.solr.cloud.ConnectionManagerStaleExpiryTest.testStaleExpiryDoesNotReplaceRecoveredSession -Ptests.seed=5301C0DE --rerun-tasks ``` -- 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]
