Add primitive method allows rollback a single operation #31910
Conversation
This change introduces a new primitive method which supports undoing a single operation. This utility will be used to build Lucene rollback.
dnhatn
added
>feature
:Distributed Indexing/Engine
|
Pinging @elastic/es-distributed |
|
/cc @ywelsch |
|
@bleskes I've updated the PR to introduce |
| * | ||
| * @return {@code true} if there is a difference between the colliding operation and the new operation | ||
| */ | ||
| public abstract boolean maybeRollback(MapperService mapperService, Engine.Operation newOp) throws IOException; |
There was a problem hiding this comment.
this can be private / package private. It can also be an InternalEngine thing. It's an implementation detail IMO.
There was a problem hiding this comment.
I thought we would use this from IndexShard. I removed this method.
| long collidingTerm = Lucene.readNumericDV(collidingSegment.reader(), SeqNoFieldMapper.PRIMARY_TERM_NAME, collidingDocId); | ||
| if (collidingTerm == newOp.primaryTerm()) { | ||
| // matches with the existing doc | ||
| localCheckpointTracker.markSeqNoAsCompleted(newOp.seqNo()); |
There was a problem hiding this comment.
this shouldn't be done here - it's part of the indexing logic.
| return new SubReaderWithLiveDocs(leaf, null, leaf.maxDoc()); | ||
| } | ||
| int rollbackCount = 0; | ||
| FixedBitSet liveDocs = new FixedBitSet(leaf.maxDoc()); |
There was a problem hiding this comment.
I am exploring a cache of liveDocs using coreCacheHelper and docValuesGeneration as key. It looks good, but I need to make sure that it works correctly and write some tests.
There was a problem hiding this comment.
I think you don't need to create cache keys and could directly use LeafReader instances as cache keys.
| searcher.setQueryCache(null); | ||
| final FieldsVisitor fields; | ||
| final Query currentVersionQuery = LongPoint.newExactQuery(SeqNoFieldMapper.NAME, newOp.seqNo()); | ||
| try (ReleasableLock ignored = readLock.acquire()) { |
There was a problem hiding this comment.
I think this should be done on the top level try no? before we call acquireSearcher.
|
@dhnat and I discussed some more potential simplifications via another channel. |
|
@dnhatn I think that we can close this PR for now? |
|
@jasontedor Yes. |
5 participants
This change introduces a new primitive method which supports undoing a single operation. This utility will be used to build Lucene rollback.