Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync retention leases on expiration #37902

Merged
merged 3 commits into from
Jan 28, 2019

Conversation

jasontedor
Copy link
Member

This commit introduces a sync of retention leases when a retention lease expires. As expiration of retention leases is lazy, their expiration is managed only when getting the current retention leases from the replication tracker. At this point, we callback to our full retention lease sync to sync and flush these on all shard copies. With this change, replicas do not locally manage expiration of retention leases; instead, that is done only on the primary.

Relates #37165

This commit introduces a sync of retention leases when a retention lease
expires. As expiration of retention leases is lazy, their expiration is
managed only when getting the current retention leases from the
replication tracker. At this point, we callback to our full retention
lease sync to sync and flush these on all shard copies. With this
change, replicas do not locally manage expiration of retention leases;
instead, that is done only on the primary.
@jasontedor jasontedor added >enhancement v7.0.0 :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. v6.7.0 labels Jan 27, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@jasontedor jasontedor mentioned this pull request Jan 27, 2019
24 tasks
@dnhatn
Copy link
Member

dnhatn commented Jan 27, 2019

If a replica executes retention-leases sync requests out of order, it may have a different set of retention-leases than the primary. Suppose the primary has no retention lease, then we add two retention-leases L1 and L2. We will execute two sync requests with {L1} and {L1, L2}. If the second request is executed before the first request on a replica, the replica will have one lease {L1} while the primary has two leases: {L1 and L2}. We may need to add primary-term and seqId to the retention-leases sync requests. We can address this either before or after this PR.

@jasontedor
Copy link
Member Author

@dnhatn Yes, we will need to add a version to it. Primary term/sequence number is insufficient because there’s no correlation between when a retention lease is taken and whether or not the sequence number has advanced. We will instead need our own version number. I want to do this in a follow up after the background sync is in too since it will touch all the syncing code.

@dnhatn
Copy link
Member

dnhatn commented Jan 28, 2019

@jasontedor I meant a version number like you said :). I think we need to add the primary-term to reject the sync requests that from the demoted primary.

@jasontedor
Copy link
Member Author

I think we need to add the primary-term to reject the sync requests that from the demoted primary.

I don't think this is needed since these are replication actions, so rejection based on the primary term is already handled for us.

Copy link
Member

@dnhatn dnhatn left a comment

Choose a reason for hiding this comment

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

LGTM.

@dnhatn
Copy link
Member

dnhatn commented Jan 28, 2019

I don't think this is needed since these are replication actions, so rejection based on the primary term is already handled for us.

Ah, I found it. Thanks for the explanation. Let's add the version to the sync request.

@jasontedor jasontedor merged commit 194cdfe into elastic:master Jan 28, 2019
jasontedor added a commit that referenced this pull request Jan 28, 2019
This commit introduces a sync of retention leases when a retention lease
expires. As expiration of retention leases is lazy, their expiration is
managed only when getting the current retention leases from the
replication tracker. At this point, we callback to our full retention
lease sync to sync and flush these on all shard copies. With this
change, replicas do not locally manage expiration of retention leases;
instead, that is done only on the primary.
@jasontedor jasontedor deleted the sync-retention-lease-expiration branch January 28, 2019 12:18
@jasontedor
Copy link
Member Author

I opened #37951.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. >enhancement v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants