-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Track max seq_no of updates or deletes on primary #33842
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
Changes from all commits
587ea79
2297846
d84de4a
1ffeaea
f27d5f8
45fc088
a7269dc
2dd8758
dcc23fd
516fbeb
94fa692
e92e71e
fbc4de2
fafd679
0b632a3
84a985f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -385,6 +385,7 @@ public InternalEngine recoverFromTranslog(TranslogRecoveryRunner translogRecover | |
| flushLock.lock(); | ||
| try (ReleasableLock lock = readLock.acquire()) { | ||
| ensureOpen(); | ||
| assert getMaxSeqNoOfUpdatesOrDeletes() != SequenceNumbers.UNASSIGNED_SEQ_NO : "max_seq_no_of_updates is uninitialized"; | ||
| if (pendingTranslogRecovery.get() == false) { | ||
| throw new IllegalStateException("Engine has already been recovered"); | ||
| } | ||
|
|
@@ -918,6 +919,7 @@ protected IndexingStrategy indexingStrategyForOperation(final Index index) throw | |
|
|
||
| protected final IndexingStrategy planIndexingAsPrimary(Index index) throws IOException { | ||
| assert index.origin() == Operation.Origin.PRIMARY : "planing as primary but origin isn't. got " + index.origin(); | ||
| assert getMaxSeqNoOfUpdatesOrDeletes() != SequenceNumbers.UNASSIGNED_SEQ_NO : "max_seq_no_of_updates is not initialized"; | ||
| final IndexingStrategy plan; | ||
| // resolve an external operation into an internal one which is safe to replay | ||
| if (canOptimizeAddDocument(index)) { | ||
|
|
@@ -952,6 +954,10 @@ protected final IndexingStrategy planIndexingAsPrimary(Index index) throws IOExc | |
| ); | ||
| } | ||
| } | ||
| final boolean toAppend = plan.indexIntoLucene && plan.useLuceneUpdateDocument == false; | ||
| if (toAppend == false) { | ||
| advanceMaxSeqNoOfUpdatesOrDeletes(plan.seqNoForIndexing); | ||
| } | ||
| return plan; | ||
| } | ||
|
|
||
|
|
@@ -1242,6 +1248,7 @@ protected boolean assertNonPrimaryOrigin(final Operation operation) { | |
|
|
||
| protected final DeletionStrategy planDeletionAsPrimary(Delete delete) throws IOException { | ||
| assert delete.origin() == Operation.Origin.PRIMARY : "planing as primary but got " + delete.origin(); | ||
| assert getMaxSeqNoOfUpdatesOrDeletes() != SequenceNumbers.UNASSIGNED_SEQ_NO : "max_seq_no_of_updates is not initialized"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment about index version and bwc.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. commenting here as I can comment on the relevant lines - can we add an assertion in the index method and delete method that if we update/delete lucene than the seq# of the delete/update is <= msu?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess you need the replica logic to be in place for that assertions to be added.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we need to replicate msu first. I will add these assertions later.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We make sure that we always initialize max_seq_no_of_updates of the engine of a primary shard (in IndexShard). Please let me know if it's correct. |
||
| // resolve operation from external to internal | ||
| final VersionValue versionValue = resolveDocVersion(delete); | ||
| assert incrementVersionLookup(); | ||
|
|
@@ -1263,6 +1270,7 @@ protected final DeletionStrategy planDeletionAsPrimary(Delete delete) throws IOE | |
| currentlyDeleted, | ||
| generateSeqNoForOperation(delete), | ||
| delete.versionType().updateVersion(currentVersion, delete.version())); | ||
| advanceMaxSeqNoOfUpdatesOrDeletes(plan.seqNoOfDeletion); | ||
| } | ||
| return plan; | ||
| } | ||
|
|
@@ -2548,4 +2556,12 @@ private void updateAutoIdTimestamp(long newTimestamp, boolean unsafe) { | |
| assert maxUnsafeAutoIdTimestamp.get() <= maxSeenAutoIdTimestamp.get(); | ||
| } | ||
|
|
||
|
|
||
| @Override | ||
| public void initializeMaxSeqNoOfUpdatesOrDeletes() { | ||
| assert getMaxSeqNoOfUpdatesOrDeletes() == SequenceNumbers.UNASSIGNED_SEQ_NO : | ||
dnhatn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| "max_seq_no_of_updates is already initialized [" + getMaxSeqNoOfUpdatesOrDeletes() + "]"; | ||
| final long maxSeqNo = SequenceNumbers.max(localCheckpointTracker.getMaxSeqNo(), translog.getMaxSeqNo()); | ||
| advanceMaxSeqNoOfUpdatesOrDeletes(maxSeqNo); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may cause a bwc issue - if the primary is on an old node, it's a problem. Maybe only assert if the index created version is high enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We make sure that we always initialize max_seq_no_of_updates of the engine of a primary shard (in IndexShard). Please let me know if it's correct.