Skip to content

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented Mar 26, 2019

Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send operations without sequence number to a replica which already processed operations with sequence number. This leads to the failure of that replica for we trip the sequence number assertion when writing resync operations without sequence number to translog.

@dnhatn dnhatn added >bug WIP :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v7.0.0 v8.0.0 v7.2.0 v6.7.1 labels Mar 26, 2019
@dnhatn dnhatn requested a review from bleskes March 26, 2019 02:20
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@dnhatn dnhatn requested a review from ywelsch March 26, 2019 02:20
@bleskes
Copy link
Contributor

bleskes commented Mar 26, 2019

@dnhatn I think I'm missing something. The resync is in charge of verifying operations above the global checkpoint. We currently starting to sync from the global checkpoint + 1 , which seems to only be negative if the global checkpoint is uknown on the new primary. I think we talked before (although I don't remember the details) about bootstrapping the global checkpoint in these cases. I think the right solution is there (and also asserting we always have a starting sequence number for a resync). Is there any reason for you to go down the route of explicit filtering rather than checking the starting sequence number?

@dnhatn
Copy link
Member Author

dnhatn commented Mar 26, 2019

@bleskes Thank you for looking :)

Bootstrapping the global checkpoint would work too - I can make that change. I did not have any preference for my implementation, merely followed the pattern we did in #27580. Moreover, if we have an assigned global checkpoint then we will never use operations without seqno; therefore I think we should exclude them from translog snapshot.

@bleskes
Copy link
Contributor

bleskes commented Mar 27, 2019

Bootstrapping the global checkpoint would work too - I can make that change. I did not have any preference for my implementation, merely followed the pattern we did in #27580

I'm not sure I follow the reference to the PR as it's about flushing, but that aside - anything we can do to make the situation as "normal" as possible the better it is. Under normal operation (I.e., no BWC mode) we always have a global checkpoint, so that's why I favor that direction.

@ywelsch
Copy link
Contributor

ywelsch commented Mar 29, 2019

There is another related issue here: https://discuss.elastic.co/t/extremely-large-translog-files-per-shard-in-elasticsearch-6-2-4/174041/5 as a gcp of UNASSIGNED_SEQ_NO can cause a resync to send the full translog, which can cause high loads during a rolling upgrade.

Bootstrapping the global checkpoint would work too - I can make that change.

I'm a little uncomfortable making that change in 6.7.1, as it might have a broader impact, especially in the presence of 5.x indices with no sequence numbers. I wonder if we can have the more contained change here for now to be backported to 6.7 and follow-up with a more comprehensive investigation of getting rid of UNASSIGNED_SEQ_NO in various places as we know that in 7.x all docs will have sequence numbers. WDYT @bleskes?

@bleskes
Copy link
Contributor

bleskes commented Mar 29, 2019

WDYT @bleskes?
+1

@colings86 colings86 added v6.7.2 and removed v6.7.1 labels Mar 30, 2019
@dnhatn dnhatn changed the title Primary-replica resync should not send ops without seqno Resync should not send ops if global checkpoint unassigned Mar 31, 2019
@dnhatn dnhatn removed the WIP label Mar 31, 2019
@dnhatn
Copy link
Member Author

dnhatn commented Mar 31, 2019

@bleskes and @ywelsch This is ready again. Can you please have another look? Thank you!

@dnhatn dnhatn requested review from bleskes and removed request for bleskes and ywelsch March 31, 2019 01:57
@dnhatn dnhatn requested a review from ywelsch March 31, 2019 01:57
@dnhatn dnhatn changed the title Resync should not send ops if global checkpoint unassigned Resync should not send operations without sequence number Apr 3, 2019
@dnhatn
Copy link
Member Author

dnhatn commented Apr 3, 2019

@ywelsch I pushed 61ad34d to use a weaker form which won't send operations without sequence numbers. Can you have another look? Thank you!

@dnhatn dnhatn requested a review from ywelsch April 3, 2019 00:53
Copy link
Contributor

@ywelsch ywelsch 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 Author

dnhatn commented Apr 4, 2019

Thanks @bleskes and @ywelsch.

@dnhatn dnhatn merged commit c737943 into elastic:master Apr 4, 2019
@dnhatn dnhatn deleted the resync-ops branch April 4, 2019 02:17
dnhatn added a commit that referenced this pull request Apr 4, 2019
Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send
operations without sequence number to a replica which already processed
operations with sequence number. This leads to the failure of that
replica for we trip the sequence number assertion when writing resync
operations without sequence number to translog.
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Apr 5, 2019
Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send
operations without sequence number to a replica which already processed
operations with sequence number. This leads to the failure of that
replica for we trip the sequence number assertion when writing resync
operations without sequence number to translog.
dnhatn added a commit that referenced this pull request Apr 5, 2019
Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send
operations without sequence number to a replica which already processed
operations with sequence number. This leads to the failure of that
replica for we trip the sequence number assertion when writing resync
operations without sequence number to translog.
dnhatn added a commit that referenced this pull request Apr 5, 2019
Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send
operations without sequence number to a replica which already processed
operations with sequence number. This leads to the failure of that
replica for we trip the sequence number assertion when writing resync
operations without sequence number to translog.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Primary-replica resync in a mixed-cluster between 6.x and 5.6 can send
operations without sequence number to a replica which already processed
operations with sequence number. This leads to the failure of that
replica for we trip the sequence number assertion when writing resync
operations without sequence number to translog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed Indexing/Recovery Anything around constructing a new shard, either from a local or a remote source. v6.7.2 v7.0.0 v7.2.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants