Skip to content

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented Nov 26, 2017

Today we can not distinguish between index commits that are kept by the
primary policy and those are kept for snapshotting with a
SnapshotDeletionPolicy. Since we enclose a SnapshotDeletionPolicy in a
CombinedDeletionPolicy, we also we can not distinguish between those
with a CombinedDeletionPolicy. This can be a problem if we update the
TranslogDeletionPolicy to keep the minimum translog generation of
undeleted index commits as we may keep the translog of a snapshotting
commit even though it is "deleted" by the primary policy.

To solve this, we enclose a CombinedDeletionPolicy in a
SnapshotDeletionPolicy and track if an index commit is deleted by the
primary policy, then use that value to maintain translog rather than the
actual deletion of an index commit.

Relates #27456 #27367

Today we can not distinguish between index commits that are kept by the
primary policy and those are kept for snapshotting with a
SnapshotDeletionPolicy. Since we enclose a SnapshotDeletionPolicy in a
CombinedDeletionPolicy, we also we can not distinguish between those
with a CombinedDeletionPolicy. This can be a problem if we update the
TranslogDeletionPolicy to keep the minimum translog generation of
undeleted index commits as we may keep the translog of a snapshotting
commit even though it is "deleted" by the primary policy.

To solve this, we enclose a CombinedDeletionPolicy in a
SnapshotDeletionPolicy and track if an index commit is deleted by the
primary policy, then use that value to maintain translog rather than the
actual deletion of an index commit.

Relates elastic#27456 elastic#27367
@dnhatn dnhatn requested a review from s1monw November 27, 2017 02:34
* Wraps a provided {@link IndexCommit} and tracks if it has been deleted by the policy.
* This tracks if {@link #delete()} has been called or not even the actual action can be ignored by {@link SnapshotDeletionPolicy}.
*/
private static class TrackedIndexCommit extends IndexCommit {
Copy link
Contributor

Choose a reason for hiding this comment

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

This sucks.. I want to see how big of a deal it is to keep things as they were. Indeed snapshotting a commit will keep it's translog around but I'm not sure anymore it's worth this kind of wrapping layers. Maybe we should invest in faster clean up on those snapshotted commits. I'll reach out to discuss.

@dnhatn
Copy link
Member Author

dnhatn commented Nov 27, 2017

This sucks..

I am closing this and will integrate this policy into the KeepUntilGlobalCheckpointDeletionPolicy.

@dnhatn dnhatn closed this Nov 27, 2017
@dnhatn dnhatn deleted the snapshot-policy branch November 27, 2017 17:20
@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 :Translog :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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants