SOLR-18406: Restart replication after index generation expires - #4820
Open
JHSUYU wants to merge 1 commit into
Open
SOLR-18406: Restart replication after index generation expires#4820JHSUYU wants to merge 1 commit into
JHSUYU wants to merge 1 commit into
Conversation
Author
|
Hi @HoustonPutman , could you take a look when you are available? Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SOLR-18406: Restart replication after index generation expires
Description
During leader/follower index replication, the follower selects generation G, requests filelist(G), and then downloads its files through separate filecontent requests. Although there is no explicit replication session, G acts as the consistency boundary for the complete replication cycle. The leader pins G while each request is active, but between requests it is protected only by commitReserveDuration—10 seconds by default.
This bounded reservation is intentional: if a follower crashes midway through replication, it must not retain old commits indefinitely. However, the leader cannot distinguish a crashed follower from a live follower delayed by GC, networking, disk I/O, or scheduling. If such a delay exceeds the reservation and a subsequent leader commit deletes G, the follower may return with a filecontent request for a generation that is no longer available.
The earlier indexversion → filelist handoff already handles this stale-generation condition. If G is deleted after indexversion returns it but before filelist(G) begins, getFileList() catches the IllegalStateException, reports "invalid index generation", and returns no file list. The follower interprets the missing list as an empty download set and terminates the current attempt with PEER_INDEX_COMMIT_DELETED, without downloading files from G. A later replication attempt can then begin with a new indexversion request.
The subsequent filelist → filecontent handoff had no equivalent handling. DirectoryFileStream.initWrite() called saveCommitPoint(G) directly, allowing the same IllegalStateException to escape as HTTP 500. The follower treated it as an ordinary file-transfer failure, retried the file against the same unavailable generation, and then aborted the entire replication cycle, wasting any files already downloaded during that cycle.
This appears to expose a protocol-level gap (correct me if I am wrong): generation G is a session-level consistency boundary—the follower expects to use it for the complete replication cycle—but the leader protects it using request-level reservations. The short reservation is intentional so that a follower that crashes cannot retain old commits indefinitely. However, the leader cannot distinguish a crashed follower from a live follower delayed between
requests, so an active replication can legitimately outlive its reservation. Generation expiration must therefore be treated as an expected protocol event that invalidates the current snapshot and requires renegotiation. Returning a generic HTTP 500 provided no such state transition, causing the follower to retry an irrecoverably stale generation instead of restarting from the leader’s current generation.
Solution
Convert the stale-generation failure in
DirectoryFileStream.initWrite()into an explicit HTTP 409 conflict with aninvalid index generationmessage.When
IndexFetcherreceives this response, it stops retrying the same file from the already unavailable generation and restarts the complete replication cycle once, beginning with a newindexversionrequest. This allows the follower to fetch the leader's current generation instead of failing the entire replication attempt.The change also ensures that a commit point is only reserved and released when it was successfully saved.
Tests
Added
TestReplicationHandler.testFollowerRestartsWhenCommitExpiresBeforeFileDownload.The test uses real leader and follower Jetty instances and performs this sequence:
fetchindexrequest.filelist(G)and before requestingfilecontent(G).