Introduce retention lease background sync#38262
Merged
jasontedor merged 5 commits intoelastic:masterfrom Feb 4, 2019
Merged
Conversation
This commit introduces a background sync for retention leases. The idea here is that we do a heavyweight sync when adding a new retention lease, and then periodically we want to background sync any retention lease renewals to the replicas. As long as the background sync interval is significantly lower than the extended lifetime of a retention lease, it is okay if from time to time a replica misses a sync (it will still have an older version of the lease that is retaining more data as we assume that renewals do not decrease the retaining sequence number). There are two follow-ups that will come after this commit. The first is to address the fact that we have not adapted the should periodically flush logic to possibly flush the retention leases. We want to do something like flush if we have not flushed in the last five minutes and there are renewed retention leases since the last time that we flushed. An additional follow-up will remove the syncing of retention leases when a retention lease expires. Today this sync could be invoked in the background by a merge operation. Rather, we will move the syncing of retention lease expiration to be done under the background sync. The background sync will use the heavyweight sync (write action) if a lease has expired, and will use the lightweight background sync (replication action) otherwise.
jasontedor
added
>enhancement
v7.0.0
:Distributed/Distributed
Collaborator
|
Pinging @elastic/es-distributed |
This was referenced Feb 2, 2019
dnhatn
reviewed
Feb 3, 2019
Member
dnhatn
left a comment
dnhatn
left a comment
There was a problem hiding this comment.
Looks great. I left a point to discuss.
server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseBackgroundSyncAction.java
Show resolved
Hide resolved
dnhatn
approved these changes
Feb 4, 2019
Member
dnhatn
left a comment
dnhatn
left a comment
There was a problem hiding this comment.
I left two minor comments. LGTM.
| protected PrimaryResult<Request, ReplicationResponse> shardOperationOnPrimary(final Request request, final IndexShard primary) { | ||
| Objects.requireNonNull(request); | ||
| Objects.requireNonNull(primary); | ||
| primary.afterWriteOperation(); |
Member
There was a problem hiding this comment.
I am not sure if this call is useful for it does not change translog nor Lucene.
| Objects.requireNonNull(request); | ||
| Objects.requireNonNull(replica); | ||
| replica.updateRetentionLeasesOnReplica(request.getRetentionLeases()); | ||
| replica.afterWriteOperation(); |
Member
There was a problem hiding this comment.
same here - I am not sure if this call is useful for it does not change translog nor Lucene.
Member
Author
There was a problem hiding this comment.
The goal is to trigger periodic flush. Let us discuss the exact mechanics of this in a follow-up as that work develops.
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
4 participants
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.
This commit introduces a background sync for retention leases. The idea here is that we do a heavyweight sync when adding a new retention lease, and then periodically we want to background sync any retention lease renewals to the replicas. As long as the background sync interval is significantly lower than the extended lifetime of a retention lease, it is okay if from time to time a replica misses a sync (it will still have an older version of the lease that is retaining more data as we assume that renewals do not decrease the retaining sequence number). There are two follow-ups that will come after this commit. The first is to address the fact that we have not adapted the should periodically flush logic to possibly flush the retention leases. We want to do something like flush if we have not flushed in the last five minutes and there are renewed retention leases since the last time that we flushed. An additional follow-up will remove the syncing of retention leases when a retention lease expires. Today this sync could be invoked in the background by a merge operation. Rather, we will move the syncing of retention lease expiration to be done under the background sync. The background sync will use the heavyweight sync (write action) if a lease has expired, and will use the lightweight background sync (replication action) otherwise.
Relates #37165