Index stale operations to Lucene to have complete history #29679

dnhatn · 2018-04-24T18:33:01Z

Today, when processing out of order operations, we only add it into
translog but skip adding into Lucene. Translog, therefore, has a
complete history of sequence numbers while Lucene does not.

Since we would like to have a complete history in Lucene, this change
makes sure that stale operations will be added to Lucene as soft-deleted
documents if required.

Today, when processing out of order operations, we only add it into translog but skip adding into Lucene. Translog, therefore, has a complete history of sequence numbers while Lucene does not. Since we would like to have a complete history in Lucene, this change makes sure that stale operations will be added to Lucene as soft-deleted documents if required.

dnhatn · 2018-04-24T18:34:45Z

@s1monw and @bleskes Can you please have a look? I could not make review requests. Thank you!

elasticmachine · 2018-04-24T22:13:20Z

Pinging @elastic/es-distributed

s1monw

I left some minor things. this looks pretty awesome! @bleskes please take a look at the engine parts.

s1monw · 2018-04-25T12:56:41Z

server/src/main/java/org/elasticsearch/common/lucene/Lucene.java

+     * @param in the input directory reader
+     * @return the wrapped reader including soft-deleted documents.
+     */
+    public static DirectoryReader includeSoftDeletes(DirectoryReader in) throws IOException {


I think we should just call wrapAllDocsLive(DirectoryReader in) it's really unrelated to soft deletes

s1monw · 2018-04-25T12:58:04Z

server/src/main/java/org/elasticsearch/common/lucene/uid/PerThreadIDVersionAndSeqNoLookup.java

+    /**
+     * Returns an internal posting list of the given uid
+     */
+    PostingsEnum getPostingsOrNull(BytesRef id) throws IOException {


can we use this method here as well?

s1monw · 2018-04-25T13:07:55Z

server/src/main/java/org/elasticsearch/common/lucene/uid/VersionsAndSeqNoResolver.java

+            if (postingsEnum == null) {
+                continue;
+            }
+            final NumericDocValues seqNoDV = leaf.reader().getNumericDocValues(SeqNoFieldMapper.NAME);


I think we can assert that both seqNoDV and primaryTermDV are non null. They are required at least for this code to work.

s1monw · 2018-04-25T13:08:37Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

        /** no doc was found in lucene */
        LUCENE_DOC_NOT_FOUND
    }

+    private OpVsLuceneDocStatus compareToLuceneHistory(final Operation op, final Searcher searcher) throws IOException {


can we have a javadoc for this method?

s1monw · 2018-04-25T13:10:15Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

-        /** the op is older or the same as the one that last modified the doc found in lucene*/
-        OP_STALE_OR_EQUAL,
+        /** the op is stale but its history is existed in Lucene */
+        OP_STALE_HISTORY_EXISTED,


maybe OP_STALE_HISTORY_EXISTS ?

s1monw · 2018-04-25T13:11:29Z

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

@@ -610,10 +625,10 @@ private OpVsLuceneDocStatus compareOpToLuceneDocBasedOnSeqNo(final Operation op)
                    if (op.primaryTerm() > existingTerm) {
                        status = OpVsLuceneDocStatus.OP_NEWER;
                    } else {
-                        status = OpVsLuceneDocStatus.OP_STALE_OR_EQUAL;


can you please add comments here in what cases we can get into these situations?

bleskes · 2018-04-25T20:27:20Z

I talked to @dnhatn and I'm a bit uncomfortable with the PR changes to how we resolve stale operations in the engine. It seems that the main goal here was to try to avoid indexing duplicates of the same stale operation into Lucene. I'm not sure we need to do this. This is very rare. The alternative would be to accept duplicates under the condition that duplicate seq# also means the same doc version. A few things that are worth noting on top of it:

The Translog already has this property and may have duplicates.
To fully achieve these semantics we need to implement lucene roll backs.

If we accept the above, we can keep the current (simpler) way we resolve stale operations and follow the following:

If an incoming op with a seq# < local checkpoint, skip lucene and only store in the translog (as now)
If not, resolve the latest seq# for the doc id to decide if the op is stale (as today)
Stale ops with seq# > local checkpoint go into lucene with the soft delete flag "on".

dnhatn · 2018-04-25T21:41:31Z

The alternative would be to accept duplicates under the condition that duplicate seq# also means the same doc version.

@bleskes I pushed 70d5359 to back out the dedup logic as we discussed. Can you please have another look? Thank you!

s1monw

left a tiny comment LGTM

s1monw · 2018-04-27T11:17:13Z

server/src/main/java/org/elasticsearch/common/lucene/Lucene.java

@@ -833,6 +834,58 @@ public int length() {
        };
    }

+    /**
+     * Wraps a directory reader to include all live docs.
+     * The wrapped reader can be used to query documents which are soft-deleted.


just say to query all documents

dnhatn · 2018-04-27T15:44:34Z

@elasticmachine test this please.

dnhatn · 2018-04-27T22:13:18Z

run sample packaging tests

dnhatn · 2018-04-27T22:20:00Z

@elasticmachine run sample packaging tests

dnhatn · 2018-04-27T23:38:50Z

Thanks @simonw and @bleskes for reviewing.

Today, when processing out of order operations, we only add it into translog but skip adding into Lucene. Translog, therefore, has a complete history of sequence numbers while Lucene does not. Since we would like to have a complete history in Lucene, this change makes sure that stale operations will be added to Lucene as soft-deleted documents if required. Relates #29530

Since elastic#29679 we started adding stale operations to Lucene to have a complete history in Lucene. As the stale docs are rare, we accepted to have duplicate copies of them to keep an engine simple. However, we now need to make sure that we have a single copy per stale operation in Lucene because the Lucene rollback requires a single document for each sequence number.

dnhatn requested review from bleskes and s1monw April 24, 2018 22:13

dnhatn added the :Distributed Indexing/Engine Anything around managing Lucene and the Translog in an open shard. label Apr 24, 2018

dnhatn added >enhancement :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features and removed :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features labels Apr 24, 2018

s1monw suggested changes Apr 25, 2018

View reviewed changes

dnhatn added 2 commits April 25, 2018 15:37

Merge remote-tracking branch 'elastic/ccr' into lucene-ops-history

d19ef6a

Simon’s feedbacks

5f6e851

Accept duplicate stale docs

70d5359

dnhatn added the review label Apr 25, 2018

dnhatn requested a review from s1monw April 25, 2018 21:41

Merge branch 'ccr' into lucene-ops-history

0f0a755

s1monw approved these changes Apr 27, 2018

View reviewed changes

dnhatn added 2 commits April 27, 2018 09:53

Merge branch 'ccr' into lucene-ops-history

5916467

comment for wrapped reader

687adc7

dnhatn merged commit 8ebca76 into elastic:ccr Apr 27, 2018

dnhatn deleted the lucene-ops-history branch April 27, 2018 23:39

dnhatn added the backport pending label Apr 27, 2018

dnhatn mentioned this pull request Apr 27, 2018

Use soft-deletes to maintain document history #29530

Closed

14 tasks

dnhatn removed the backport pending label May 10, 2018

dnhatn mentioned this pull request Jul 4, 2018

Do not add multiple copies of stale docs to Lucene #31806

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index stale operations to Lucene to have complete history #29679

Index stale operations to Lucene to have complete history #29679

dnhatn commented Apr 24, 2018

dnhatn commented Apr 24, 2018

elasticmachine commented Apr 24, 2018

s1monw left a comment

s1monw Apr 25, 2018

s1monw Apr 25, 2018

s1monw Apr 25, 2018

s1monw Apr 25, 2018

s1monw Apr 25, 2018

s1monw Apr 25, 2018

bleskes commented Apr 25, 2018

dnhatn commented Apr 25, 2018

s1monw left a comment

s1monw Apr 27, 2018

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018

Index stale operations to Lucene to have complete history #29679

Index stale operations to Lucene to have complete history #29679

Conversation

dnhatn commented Apr 24, 2018

dnhatn commented Apr 24, 2018

elasticmachine commented Apr 24, 2018

s1monw left a comment

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

s1monw Apr 25, 2018

Choose a reason for hiding this comment

bleskes commented Apr 25, 2018

dnhatn commented Apr 25, 2018

s1monw left a comment

Choose a reason for hiding this comment

s1monw Apr 27, 2018

Choose a reason for hiding this comment

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018

dnhatn commented Apr 27, 2018