Renew retention lease with the last known synced checkpoint #18
Conversation
This file contains 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
gbbafna
reviewed
Jun 18, 2021
| @@ -138,7 +140,8 @@ class ShardReplicationTask(id: Long, type: String, action: String, description: | |||
| rateLimiter.release() | |||
| continue | |||
| } | |||
| retentionLeaseHelper.renewRetentionLease(remoteShardId, seqNo, followerShardId) | |||
| //renew retention lease with global checkpoint so that any shard that picks up shard replication task has data until then. | |||
| retentionLeaseHelper.renewRetentionLease(remoteShardId, indexShard.lastSyncedGlobalCheckpoint, followerShardId) | |||
There was a problem hiding this comment.
Why are we using localCheckpoint at one place and GlobalCheckpoint at other ?
GlobalCheckpoint can lag behind localCheckpoint and give exception RetentionLeaseInvalidRetainingSeqNoException in that case if we try to renew existing lease with lesser id
There was a problem hiding this comment.
Ack changed this to GCP
amkhar
reviewed
Jun 19, 2021
| var seqNo = indexShard.localCheckpoint + 1 | ||
| // Adding retention lease at local checkpoint of a node. This makes sure | ||
| // new tasks spawned after node changes/shard movements are handled properly | ||
| log.info("Adding retentionlease at follower Sequence number: ${indexShard.localCheckpoint}") |
There was a problem hiding this comment.
Minor: small s in "Sequence" word ?
gbbafna
previously approved these changes
Jun 23, 2021
gbbafna
approved these changes
Jun 23, 2021
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.
Renew retention lease with the last known synced checkpoint
Renew retention lease with the last known synced checkpoint
Issues Resolved
when follower node which has primary shard for an index is down, a replica shard picks up the task. In this case previous code always used to add retention lease with -1 sequence number leading to replication failure.
This change makes sure that the last checkpoint on a follower node is used to add retention lease and hence replication will resume gracefully. Following logs when a node is down shows that replication is resumed gracefully
Logs in Follower primary shard's node when node was terminated
Logs in follower node which is selected as new primary where replication is resumed gracefully