Remove versionType from translog #31945

dnhatn · 2018-07-11T00:56:04Z

With the introduction of sequence number, we no longer use versionType to
resolve out of order collision in replication and recovery requests.

This PR removes removes the versionType from translog. We can only remove
it in 7.0 because it is still required in a mixed cluster between 6.x and 5.x.

With the presence of sequence number, we no longer use versionType to resolve out of order collision in replication and recovery requests. However, we can only remove the versionType from translog in 7.0+ as it is required in a mixed cluster between 6.x and 5.x. This change relaxes the versionType check when comparing two translog operations so that a 6.x can work with a 7.x (without versionType).

elasticmachine · 2018-07-11T00:56:06Z

Pinging @elastic/es-distributed

dnhatn · 2018-07-12T11:28:17Z

I don't think we need this immediate step. I will reopen when it's ready.

bleskes

I left some minor comment for discussion

bleskes · 2018-07-13T17:56:42Z

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

        throws IOException {
-        return applyIndexOperation(seqNo, primaryTerm, version, versionType, autoGeneratedTimeStamp, isRetry,
+        return applyIndexOperation(seqNo, primaryTerm, version, VersionType.EXTERNAL, autoGeneratedTimeStamp, isRetry,


maybe it's better to pass null instead of VersionType.EXTERNAL? this way people may think it's the original one?

if we do that, maybe assert in the Engine.Op constructor that if the origin is not a primary, version type is null?

+1 on this suggestion

bleskes · 2018-07-13T17:56:56Z

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

-                                                             VersionType versionType) throws IOException {
-        return applyDeleteOperation(seqNo, primaryTerm, version, type, id, versionType, Engine.Operation.Origin.REPLICA);
+    public Engine.DeleteResult applyDeleteOperationOnReplica(long seqNo, long version, String type, String id) throws IOException {
+        return applyDeleteOperation(seqNo, primaryTerm, version, type, id, VersionType.EXTERNAL, Engine.Operation.Origin.REPLICA);


some comment as before

ywelsch · 2018-07-16T20:12:19Z

server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java

-                        "prvOp [" + prvOp + "], newOp [" + newOp + "]", previous.v2());
+                // we need to exclude versionType from this check because it's removed in 7.0
+                final boolean sameOp;
+                if (prvOp instanceof Translog.Index && newOp instanceof Translog.Index) {


versionType does not exist anymore in Translog.Index and Translog.Delete, so can't we just use equals on the Translog.Operation that we recreated from the bytes?

ywelsch · 2018-07-16T20:14:28Z

server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

        throws IOException {
-        return applyIndexOperation(seqNo, primaryTerm, version, versionType, autoGeneratedTimeStamp, isRetry,
+        return applyIndexOperation(seqNo, primaryTerm, version, VersionType.EXTERNAL, autoGeneratedTimeStamp, isRetry,


+1 on this suggestion

dnhatn · 2018-07-17T00:43:58Z

@bleskes and @ywelsch I have addressed your comments and pushed
d3780ee to reserve some format versions for 6.x. Can you please take another look?

ywelsch

LGTM

ywelsch · 2018-07-17T07:10:12Z

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

@@ -1168,6 +1168,8 @@ public long startTime() {
        public Index(Term uid, ParsedDocument doc, long seqNo, long primaryTerm, long version, VersionType versionType, Origin origin,
                     long startTime, long autoGeneratedIdTimestamp, boolean isRetry) {
            super(uid, seqNo, primaryTerm, version, versionType, origin, startTime);
+            assert (origin == Origin.PRIMARY && versionType != null) || (origin != Origin.PRIMARY && versionType == null) :


simpler to write (origin == Origin.PRIMARY) == (versionType != null)

ywelsch · 2018-07-17T07:47:47Z

server/src/main/java/org/elasticsearch/index/translog/Translog.java

@@ -1010,16 +1009,17 @@ public Source(BytesReference source, String routing) {
    public static class Index implements Operation {

        public static final int FORMAT_6_0 = 8; // since 6.0.0
-        public static final int FORMAT_NO_PARENT = FORMAT_6_0 + 1; // since 7.0
-        public static final int SERIALIZATION_FORMAT = FORMAT_NO_PARENT;
+        public static final int FORMAT_7_0 = FORMAT_6_0 + 5; // reserve some versions for 6.x


I don't think we should preemptively do this. If it comes to us requiring the extra versions for a change in 6.x, and we have not released 7.0, we can up the format version for the current change. If we have released 7.0 at that point, we cannot make any changes to the format anyhow.

With the presence of sequence number, we no longer use versionType to resolve out of order collision and remove it in 7.0. This PR adjusts translog to adapt that change. Relates #31945

dnhatn · 2018-07-18T01:59:41Z

Thanks @bleskes and @ywelsch for reviewing!

@return

* master: Painless: Simplify Naming in Lookup Package (#32177) Handle missing values in painless (#32207) add support for write index resolution when creating/updating documents (#31520) ECS Task IAM profile credentials ignored in repository-s3 plugin (#31864) Remove indication of future multi-homing support (#32187) Rest test - allow for snapshots to take 0 milliseconds Make x-pack-core generate a pom file Rest HL client: Add put watch action (#32026) Build: Remove pom generation for plugin zip files (#32180) Fix comments causing errors with Java 11 Fix rollup on date fields that don't support epoch_millis (#31890) Detect and prevent configuration that triggers a Gradle bug (#31912) [test] port linux package packaging tests (#31943) Revert "Introduce a Hashing Processor (#31087)" (#32178) Remove empty @return from JavaDoc Adjust SSLDriver behavior for JDK11 changes (#32145) [test] use randomized runner in packaging tests (#32109) Add support for field aliases. (#32172) Painless: Fix caching bug and clean up addPainlessClass. (#32142) Call setReferences() on custom referring tokenfilters in _analyze (#32157) Fix BwC Tests looking for UUID Pre 6.4 (#32158) Improve docs for search preferences (#32159) use before instead of onOrBefore Add more contexts to painless execute api (#30511) Add EC2 credential test for repository-s3 (#31918) A replica can be promoted and started in one cluster state update (#32042) Fix Java 11 javadoc compile problem Fix CP for namingConventions when gradle home has spaces (#31914) Fix `range` queries on `_type` field for singe type indices (#31756) [DOCS] Update TLS on Docker for 6.3 (#32114) ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are Remove versionType from translog (#31945) Switch distribution to new style Requests (#30595) Build: Skip jar tests if jar disabled Painless: Add PainlessClassBuilder (#32141) Build: Make additional test deps of check (#32015) Disable C2 from using AVX-512 on JDK 10 (#32138) Build: Move shadow customizations into common code (#32014) Painless: Fix Bug with Duplicate PainlessClasses (#32110) Remove empty @param from Javadoc Re-disable packaging tests on suse boxes Docs: Fix missing example script quote (#32010) [ML] Wait for aliases in multi-node tests (#32086) [ML] Move analyzer dependencies out of categorization config (#32123) Ensure to release translog snapshot in primary-replica resync (#32045) Handle TokenizerFactory TODOs (#32063) Relax TermVectors API to work with textual fields other than TextFieldType (#31915) Updates the build to gradle 4.9 (#32087) Mute :qa:mixed-cluster indices.stats/10_index/Index - all’ Check that client methods match API defined in the REST spec (#31825) Enable testing in FIPS140 JVM (#31666) Fix put mappings java API documentation (#31955) Add exclusion option to `keep_types` token filter (#32012) [Test] Modify assert statement for ssl handshake (#32072)

Normally translog operations will not be replayed on the primary. Following engine is an exception where we replay translog on both primary and replica as a non-primary strategy. Even though we won't use the version_type in the following engine, we still need to pass a valid value for the primary operation in order not to trip assertions in an engine. This commit passes version_type EXTERNAL for translog operation if its origin is primary. Relates #31945

dnhatn added >non-issue v7.0.0 :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v6.4.0 labels Jul 11, 2018

dnhatn requested review from bleskes and ywelsch July 11, 2018 00:56

dnhatn added 2 commits July 11, 2018 10:57

Let remove this in 7.0

700eee4

Merge branch 'master' into relax-version-type

d54e5ff

dnhatn closed this Jul 12, 2018

dnhatn added 2 commits July 12, 2018 14:02

Merge branch 'master' into relax-version-type

43a1eb6

remove versionType

e58c69e

dnhatn changed the title ~~Relax versionType check in translog~~ Remove versionType from translog Jul 13, 2018

dnhatn mentioned this pull request Jul 13, 2018

Adjust translog after versionType is removed in 7.0 #32020

Merged

dnhatn added >enhancement and removed >non-issue v6.4.0 labels Jul 13, 2018

dnhatn reopened this Jul 13, 2018

dnhatn added 2 commits July 13, 2018 08:21

Merge branch 'master' into relax-version-type

4e171a4

Merge branch 'master' into relax-version-type

0a203e6

bleskes reviewed Jul 13, 2018

View reviewed changes

ywelsch reviewed Jul 16, 2018

View reviewed changes

dnhatn added 5 commits July 16, 2018 16:25

Merge branch 'master' into relax-version-type

c8b1c88

non-primary should not have version_type

badbf44

assert write version on primary

5b07b1a

use operation equals method

15e3c4d

reserve some versions for 6.x

d3780ee

ywelsch approved these changes Jul 17, 2018

View reviewed changes

dnhatn added 2 commits July 17, 2018 09:21

bwc format

b891b6b

simplify assertion

5ac734f

dnhatn merged commit df1380b into elastic:master Jul 18, 2018

dnhatn deleted the relax-version-type branch July 18, 2018 01:59

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove versionType from translog #31945

Remove versionType from translog #31945

dnhatn commented Jul 11, 2018 •

edited

Loading

elasticmachine commented Jul 11, 2018

dnhatn commented Jul 12, 2018

bleskes left a comment

bleskes Jul 13, 2018

bleskes Jul 13, 2018

ywelsch Jul 16, 2018

bleskes Jul 13, 2018

ywelsch Jul 16, 2018

ywelsch Jul 16, 2018

dnhatn commented Jul 17, 2018

ywelsch left a comment

ywelsch Jul 17, 2018

ywelsch Jul 17, 2018

bleskes Jul 17, 2018

dnhatn Jul 17, 2018

dnhatn commented Jul 18, 2018

Remove versionType from translog #31945

Remove versionType from translog #31945

Conversation

dnhatn commented Jul 11, 2018 • edited Loading

elasticmachine commented Jul 11, 2018

dnhatn commented Jul 12, 2018

bleskes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Jul 17, 2018

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Jul 18, 2018

dnhatn commented Jul 11, 2018 •

edited

Loading