Skip to content

Port Primary Terms to master#17044

Merged
bleskes merged 1 commit intoelastic:masterfrom
bleskes:primary_terms
Mar 25, 2016
Merged

Port Primary Terms to master#17044
bleskes merged 1 commit intoelastic:masterfrom
bleskes:primary_terms

Conversation

@bleskes
Copy link
Copy Markdown
Contributor

@bleskes bleskes commented Mar 10, 2016

Primary terms is a way to make sure that operations replicated from stale primary are rejected by shards following a newly elected primary.

Original PRs adding this to the seq# feature branch #14062 , #14651

Relates to #17038

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This exception will be treated as ignore replica exception. 😉

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this check is wrong. When we have relocation going on and relocation source is marked as relocated (i.e. we call executeRemotely in TransportReplicationAction), then we have primary relocation target replicating back to primary relocation source (see also ReplicationPhase).

@bleskes
Copy link
Copy Markdown
Contributor Author

bleskes commented Mar 11, 2016

@jasontedor pushed an update to those exceptions... sadly testing is harder (but will be possible with new testing infra I'm working on...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this can be reused to replace routedBasedOnClusterVersion (#16274). Yay 👍

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

interesting idea - it's currently not incremented when relocating a primary though... requires more thought.

@ywelsch
Copy link
Copy Markdown
Contributor

ywelsch commented Mar 11, 2016

Thanks @bleskes for porting this to master. Primary terms will be really helpful in many kind of resiliency-related scenarios (solved previously with ugly hacks, e.g. routedBasedOnClusterVersion (#16274)).
I left some comments and more general thoughts about primary terms. I haven't looked at src/test/** yet but can do this in a second iteration.

@bleskes
Copy link
Copy Markdown
Contributor Author

bleskes commented Mar 12, 2016

@ywelsch I pushed a new implementation. I'm starting to warm up to making shard routing immutable on the index shard level (different PR, and not now :))

@bleskes
Copy link
Copy Markdown
Contributor Author

bleskes commented Mar 13, 2016

Pushed another commit fixing a side effect of changing the exception types from IllegalShardStateException to IllegalArgumentException (to help with #17038 ) - the replica wrapper shouldn't fail shards locally anymore (technical engine level errors are still dealt with by the engine), but leave it to the primary. It was originally added in #5847 but with current machinery it's not needed.

It's also useful to note that adding assertions of the terms invariants to IndexShard forced some clean ups in IndicesClusterService (as @ywelsch already suspected).

I will update the PR description with these and whatever else we find once the review cycles are done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

opration -> operation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants