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

Fix pre-6.0 response to unknown replication actions #25744

Merged
merged 3 commits into from Jul 17, 2017

Conversation

Projects
None yet
4 participants
@jasontedor
Copy link
Member

commented Jul 17, 2017

When sending replica requests for replication operations, we skip sending the request to pre-6.0 nodes for operations that such nodes would not be aware of (e.g., the background global checkpoint sync, or the primary/replica resync) since they would not know what to do with these requests. Yet, we simulate that we received responses from these nodes. Today, this is done by simulating that they sent us that their local checkpoint is unassigned sequence number. However, for pre-6.0 nodes we have introduced a special local checkpoint used in the global checkpoint tracker for such nodes and that is what we should use here too. This commit fixes this issue.

Relates #10708

Fix pre-6.0 response to unknown replication actions
When sending replica requests for replication operations, we skip
sending the request to pre-6.0 nodes for operations that such nodes
would not be aware of (e.g., the background global checkpoint sync, or
the primary/replica resync) since they would not know what to do with
these requests. Yet, we simulate that we received responses from these
nodes. Today, this is done by simulating that they sent us that their
local checkpoint is unassigned sequence number. However, for pre-6.0
nodes we have introduced a special local checkpoint used in the global
checkpoint tracker for such nodes and that is what we should use here
too. This commit fixes this issue.
@s1monw

s1monw approved these changes Jul 17, 2017

Copy link
Contributor

left a comment

LGTM

Merge branch 'master' into replica-response-pre-6x-node
* master:
  Move more token filters to analysis-common module
  Fix another simulate example in ingest docs
* checkpoint value when simulating responses to replication actions that pre-6.0 nodes are not aware of (e.g., the global
* checkpoint background sync, and the primary/replica resync).
*/
assert localCheckpoint != SequenceNumbersService.UNASSIGNED_SEQ_NO;

This comment has been minimized.

Copy link
@ywelsch

ywelsch Jul 17, 2017

Contributor

can you also add this assertion to GlobalCheckpointTracker.updateLocalCheckpoint (the private method)? I guess that some tests will start to fail as they are not correctly exercising the tracker.

This comment has been minimized.

Copy link
@jasontedor

jasontedor Jul 17, 2017

Author Member

@ywelsch I pushed 78f1d4c, can you take another look?

@ywelsch
Copy link
Contributor

left a comment

LGTM. Thanks @jasontedor

@jasontedor jasontedor merged commit f121cd3 into elastic:master Jul 17, 2017

1 of 2 checks passed

elasticsearch-ci Build started sha1 is merged.
Details
CLA Commit author is a member of Elasticsearch
Details

@jasontedor jasontedor deleted the jasontedor:replica-response-pre-6x-node branch Jul 17, 2017

@jasontedor

This comment has been minimized.

Copy link
Member Author

commented Jul 17, 2017

Thanks @ywelsch and @s1monw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.