Add replica ops with version conflict to translog #22626
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
An operation that completed successfully on a primary can result in a version conflict on a replica due to the asynchronous nature of operations. When a replica operation results in a version conflict, the operation is not added to the translog. This leads to gaps in the translog which is problematic as it can lead to situations where a replica shard can never advance its local checkpoint. As such operations are just normal course of business for a replica shard, these operations should be treated as if they completed successfully. This commit adds these operations to the translog.
jasontedor
force-pushed
the
replica-version-conflict
branch
from
7bae49e
to
71b89ef
Compare
bleskes
reviewed
Jan 16, 2017
| @@ -489,8 +486,9 @@ public GetResult get(Get get, Function<String, Searcher> searcherFactory) throws | |||
| op.id(), | |||
| op.versionType().explainConflictForWrites(currentVersion, expectedVersion, deleted)); | |||
| result = onFailure.apply(e); | |||
| } else { | |||
| result = onSuccess.get(); | |||
There was a problem hiding this comment.
can we add a comment here ? something like "version conflicts during recovery and replica operation are normal and occur due to out of order delivery. we should return a successful result"
| @@ -804,18 +804,17 @@ private DeleteResult innerDelete(Delete delete) throws IOException { | |||
|
|
|||
| final long expectedVersion = delete.version(); | |||
|
|
|||
| final Optional<DeleteResult> result = | |||
| final Optional<DeleteResult> checkVersionConflictResult = | |||
There was a problem hiding this comment.
nit: shall we call this resultOnVersionConflict? also - it would be good to do the same in innerIndex
This commit adds a comment explaining why version conflicts during recovery and on replicas are considered normal and thus why we return a successful result.
This commit renames the local variable used to store the result of checking whether or not there is a version conflict.
|
Thanks @bleskes. |
clintongormley
added
:Distributed/Distributed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An operation that completed successfully on a primary can result in a version conflict on a replica due to the asynchronous nature of operations. When a replica operation results in a version conflict, the operation is not added to the translog. This leads to gaps in the translog which is problematic as it can lead to situations where a replica shard can never advance its local checkpoint. As such operations are just normal course of business for a replica shard, these operations should be treated as if they completed successfully. This commit adds these operations to the translog.