Skip to content

SOLR-18530: Route adds after the proxy cut in RecoveryAfterSoftCommitTest to the shard leaders first - #5098

Open
nick-boss-tech wants to merge 2 commits into
apache:mainfrom
nick-boss-tech:solr-18530-submit
Open

nick-boss-tech wants to merge 2 commits into
apache:mainfrom
nick-boss-tech:solr-18530-submit

Conversation

@nick-boss-tech

@nick-boss-tech nick-boss-tech commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖 (posted on behalf of Nick Shanin)

https://issues.apache.org/jira/browse/SOLR-18530

What happens today

An add made after the test cuts off one replica can fail, and the test stops there.

RecoveryAfterSoftCommitTest closes one replica's socket proxy, then adds more documents through the normal cloud client. That client can send an add to the cut replica. The add fails with SolrServerException: IOException occurred when talking to server at ..., and its cause is a ClosedChannelException. The test fails before its recovery check.

What this change does

After the cut, this test sends its adds to the shard leaders first.

The adds before the cut still use the normal client. The adds after it use a new client built to send updates to shard leaders first (test). Whether the leader is the only endpoint is set by the test helper's randomized directUpdatesToLeadersOnly setting, so the cut replica can remain on the list as a failover target. The leader is not cut, so the adds reach it in either setting. The leader still sends each add on to the cut replica: a failed send to a replica of this shard is logged and does not fail the add (DistributedZkUpdateProcessor, base commit). The error is returned to the client instead when the leader changed during the send (the LeaderChanged case) (DistributedZkUpdateProcessor, base commit). The document count is unchanged, so the replica still misses more updates than its update log keeps.

Our reading of the cause, which these runs do not prove: the cloud client still held a pooled connection to the cut replica's proxy. Endpoint failover in LBSolrClient moves an update to another endpoint only when it could not connect or the request was never sent (LBSolrClient, base commit). Once the update has reached the server, a ClosedChannelException is not one of the causes that makes the cloud client resend an update (CloudSolrClient, base commit).

The change touches only this test. No product code changes.

Proof

This is a test-only change. The repeated runs pass, but they do not show that the change fixes the failure.

  • RecoveryAfterSoftCommitTest with this change: 6 of 6 runs passed. Checked 2026-10-10 at 14868bc.
  • The original test: 5 of 5 runs passed. It did not fail, so there is no before-and-after result.
  • The failure is intermittent and did not appear in these runs. This change is not shown to fix it. The reasoning rests on the code path described above.

A choice to check

The adds after the cut use a leaders-first client, not a retry in the test.

The other option was to keep the normal client and retry the same add when it fails with ClosedChannelException. That would keep any-replica writes during the partition in this test, which the leaders-first client removes. The existing test CheckRetryUnrollTest already asserts that ClosedChannelException is not retried (test, base commit), and changing that rule would be a separate decision.

Was routing these adds through a leaders-first client the right call, or would you rather keep random routing and retry in the test?

Limits

The fix is not proven by these runs, and some neighboring cases are not covered.

  • The failure did not reproduce in the runs above, so the fix is not shown to remove it.
  • Other tests that cut a replica's proxy and then write through the cloud client may have the same pattern. I have not checked them. A follow-up for those is available on request.
  • Besides the adds and the soft commit succeeding, the test checks only that replicas become active. The wait after the cut allows 30 seconds. It does not check which recovery path ran. The document count is what rules out peer sync.
  • The test helper randomizes directUpdatesToLeadersOnly for this client, so the cut replica can stay on the failover list; the leader is still tried first. Setting the flag explicitly would need a fresh set of runs, since the six passing runs used the randomized setting.
  • SolrJ still does not retry a ClosedChannelException on an update once the update has reached the server. That is a separate product decision.

Changelog: none. "Pull requests that only touch documentation, tests, build files or CI configuration are exempt and pass without an entry." (dev-docs/changelog.adoc, section 4)

AI assistance

AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.


Related fixes for the Solr 10.1.0 RC3 smoke-test failures. The RC3 smoke tester failed repeatedly on a small set of tests; the fixes are:

Test-only changes are exempt from the changelog (dev-docs/changelog.adoc),
and the entry described test internals. No code or test change.
@nick-boss-tech nick-boss-tech changed the title SOLR-18530: Route adds after the proxy cut in RecoveryAfterSoftCommitTest to leaders only SOLR-18530: Route adds after the proxy cut in RecoveryAfterSoftCommitTest to the shard leaders first Oct 10, 2026
@nick-boss-tech
nick-boss-tech marked this pull request as ready for review October 11, 2026 01:11

@iamsanjay iamsanjay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nick! LGTM. Ran the equivalent change 100 times on Linux with any-replica routing forced: 16 failures before, 0 after, with recovery still exercised every run. For my own understanding I dug into it and created a visualisation for anyone a little more interested in flaky tests: https://iamsanjay.github.io/solr-notes/RecoveryAfterSoftCommitTest.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants