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

Engine: version logic on replicas should not be hard coded #23998

Merged
merged 2 commits into from
Apr 9, 2017

Conversation

bleskes
Copy link
Contributor

@bleskes bleskes commented Apr 9, 2017

The refactoring in #23711 hardcoded version logic for replica to assume monotonic versions. Sadly that's wrong for FORCE and VERSION_GTE. Instead we should use the methods in VersionType to detect conflicts.

Note - once replicas use sequence numbers for out of order delivery, this logic goes away.

The refactoring in elastic#23711 hardcoded version logic for replica to assume monotonic versions. Sadly that's
wrong for `FORCE` and `VERSION_GTE`. Instead we should use the methods in VersionType to detect conflicts.

Note - once replicas use sequence numbers for out of order delivery, this logic goes away.
version = i;
break;
case EXTERNAL_GTE:
version = randomBoolean() && i > 0 ? i - 1 : i;
Copy link
Member

Choose a reason for hiding this comment

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

Is this clearer: version = randomBoolean() ? Math.max(i - 1, 0) : i;?

Copy link
Contributor Author

@bleskes bleskes Apr 9, 2017

Choose a reason for hiding this comment

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

sure.

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM. I left one comment. Is the title of the pull request/commit message correct?

@bleskes bleskes changed the title Engine: version logic on replicas should be hard coded Engine: version logic on replicas should not be hard coded Apr 9, 2017
@bleskes
Copy link
Contributor Author

bleskes commented Apr 9, 2017

Is the title of the pull request/commit message correct?

No... it missed a not. Thanks!

@bleskes bleskes merged commit b636ca7 into elastic:master Apr 9, 2017
@bleskes bleskes deleted the version_gte_on_replicas branch April 9, 2017 20:04
bleskes added a commit that referenced this pull request Apr 10, 2017
The refactoring in #23711 hardcoded version logic for replica to assume monotonic versions. Sadly that's wrong for `FORCE` and `VERSION_GTE`. Instead we should use the methods in VersionType to detect conflicts.
@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Engine :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Engine Anything around managing Lucene and the Translog in an open shard. >non-issue v5.4.0 v6.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants