Skip to content

Conversation

@jasontedor
Copy link
Member

@jasontedor jasontedor commented Jan 15, 2017

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.

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.
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

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

LGTM. Thx @jasontedor

op.versionType().explainConflictForWrites(currentVersion, expectedVersion, deleted));
result = onFailure.apply(e);
} else {
result = onSuccess.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

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"

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed bbffd87.

final long expectedVersion = delete.version();

final Optional<DeleteResult> result =
final Optional<DeleteResult> checkVersionConflictResult =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: shall we call this resultOnVersionConflict? also - it would be good to do the same in innerIndex

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed d40aa07.

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.
@jasontedor jasontedor merged commit e6dc74f into elastic:master Jan 16, 2017
@jasontedor jasontedor deleted the replica-version-conflict branch January 16, 2017 13:08
@jasontedor
Copy link
Member Author

Thanks @bleskes.

@clintongormley clintongormley added :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. and removed :Engine :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. >enhancement v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants