fix double version increments + some improved docs

Author: bleskes

core/src/main/java/org/elasticsearch/cluster/metadata/IndexMetaData.java

@@ -285,9 +285,9 @@ public long getVersion() {
 
     /**
      * The term of the current selected primary. This is a non-negative number incremented when
-     * a primary shard is assigned after a full cluster restart or a replica shard is promoted
-     * to a primary (see {@link ShardRouting#moveToPrimary()})
-     */
+     * a primary shard is assigned after a full cluster restart (see {@link ShardRouting#initialize(java.lang.String, long)}
+     * or a replica shard is promoted to a primary (see {@link ShardRouting#moveToPrimary()}).
+     **/
     public int primaryTerm(int shardId) {
         return this.primaryTerms[shardId];
     }
@@ -632,7 +632,6 @@ public Builder index(String index) {
 
         public Builder numberOfShards(int numberOfShards) {
             settings = settingsBuilder().put(settings).put(SETTING_NUMBER_OF_SHARDS, numberOfShards).build();
-            primaryTerms = new int[numberOfShards];
             return this;
         }
 
@@ -736,13 +735,21 @@ public Builder version(long version) {
             return this;
         }
 
+        /**
+         * returns the primary term for the given shard.
+         * See {@link IndexMetaData#primaryTerm(int)} for more information.
+         */
         public int primaryTerm(int shardId) {
             if (primaryTerms == null) {
                 initializePrimaryTerms();
             }
             return this.primaryTerms[shardId];
         }
 
+        /**
+         * sets the primary term for the given shard.
+         * See {@link IndexMetaData#primaryTerm(int)} for more information.
+         */
         public Builder primaryTerm(int shardId, int primaryTerm) {
             if (primaryTerms == null) {
                 initializePrimaryTerms();
@@ -795,7 +802,6 @@ static final class Fields {
             static final XContentBuilderString PRIMARY_TERMS = new XContentBuilderString("primary_terms");
         }
 
-
         public static void toXContent(IndexMetaData indexMetaData, XContentBuilder builder, ToXContent.Params params) throws IOException {
             builder.startObject(indexMetaData.getIndex(), XContentBuilder.FieldCaseConversion.NONE);
 

core/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java

@@ -100,6 +100,9 @@ public String getIndex() {
 
     /**
      * creates a new {@link IndexRoutingTable} with all shard versions & primary terms set to the highest found.
+     * This allows incrementing {@link ShardRouting#version()} and {@link ShardRouting#primaryTerm()} where we work on
+     * the individual shards without worrying about synchronization between {@link ShardRouting} instances. This method
+     * takes care of it.
      *
      * @return new {@link IndexRoutingTable}
      */

core/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java

@@ -256,7 +256,10 @@ public boolean primary() {
     }
 
     /**
-     * Returns the term of the current primary shard for this shard. The term is incremented with every primary promotion/initial assignment
+     * Returns the term of the current primary shard for this shard.
+     * The term is incremented with every primary promotion/initial assignment.
+     *
+     * See {@link org.elasticsearch.cluster.metadata.IndexMetaData#primaryTerm(int)} for more info.
      */
     public int primaryTerm() {
         return this.primaryTerm;

core/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java

@@ -90,8 +90,7 @@ protected RoutingAllocation.Result buildChangedResult(MetaData metaData, Routing
 
     }
     protected RoutingAllocation.Result buildChangedResult(MetaData metaData, RoutingNodes routingNodes, RoutingExplanations explanations) {
-        final RoutingTable routingTable = new RoutingTable.Builder().updateNodes(routingNodes)
-                .version(routingNodes.getRoutingTable().version() + 1).build();
+        final RoutingTable routingTable = new RoutingTable.Builder().updateNodes(routingNodes).build();
         MetaData newMetaData = updateMetaDataWithRoutingTable(metaData,routingTable);
         return new RoutingAllocation.Result(true, routingTable.validateRaiseException(newMetaData), newMetaData, explanations);
     }
@@ -136,7 +135,7 @@ static MetaData updateMetaDataWithRoutingTable(MetaData currentMetaData, Routing
             }
         }
         if (metaDataBuilder != null) {
-            return metaDataBuilder.version(currentMetaData.version() + 1).build();
+            return metaDataBuilder.build();
         } else {
             return currentMetaData;
         }

core/src/test/java/org/elasticsearch/cluster/routing/RoutingTableTests.java

@@ -31,7 +31,6 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.test.ESAllocationTestCase;
-import org.junit.Before;
 import org.junit.Test;
 
 import java.util.*;
@@ -97,9 +96,8 @@ private void initPrimaries() {
         }
         this.clusterState = ClusterState.builder(clusterState).nodes(discoBuilder).build();
         RoutingAllocation.Result rerouteResult = ALLOCATION_SERVICE.reroute(clusterState);
-        this.testRoutingTable = rerouteResult.routingTable();
         assertThat(rerouteResult.changed(), is(true));
-        this.clusterState = ClusterState.builder(clusterState).routingResult(rerouteResult).build();
+        applyRerouteResult(rerouteResult);
         versionsPerIndex.keySet().forEach(this::incrementVersion);
         primaryTermsPerIndex.keySet().forEach(this::incrementPrimaryTerm);
     }
@@ -130,11 +128,25 @@ private void startInitializingShards(String index) {
         this.clusterState = ClusterState.builder(clusterState).routingTable(this.testRoutingTable).build();
         logger.info("start primary shards for index " + index);
         RoutingAllocation.Result rerouteResult = ALLOCATION_SERVICE.applyStartedShards(this.clusterState, this.clusterState.getRoutingNodes().shardsWithState(index, INITIALIZING));
-        this.clusterState = ClusterState.builder(clusterState).routingTable(rerouteResult.routingTable()).build();
-        this.testRoutingTable = rerouteResult.routingTable();
+        // TODO: this simulate the code in InternalClusterState.UpdateTask.run() we should unify this.
+        applyRerouteResult(rerouteResult);
         incrementVersion(index);
     }
 
+    private void applyRerouteResult(RoutingAllocation.Result rerouteResult) {
+        ClusterState previousClusterState = this.clusterState;
+        ClusterState newClusterState = ClusterState.builder(previousClusterState).routingResult(rerouteResult).build();
+        ClusterState.Builder builder = ClusterState.builder(newClusterState).incrementVersion();
+        if (previousClusterState.routingTable() != newClusterState.routingTable()) {
+            builder.routingTable(RoutingTable.builder(newClusterState.routingTable()).version(newClusterState.routingTable().version() + 1).build());
+        }
+        if (previousClusterState.metaData() != newClusterState.metaData()) {
+            builder.metaData(MetaData.builder(newClusterState.metaData()).version(newClusterState.metaData().version() + 1));
+        }
+        this.clusterState = builder.build();
+        this.testRoutingTable = rerouteResult.routingTable();
+    }
+
     private void failSomePrimaries(String index) {
         this.clusterState = ClusterState.builder(clusterState).routingTable(this.testRoutingTable).build();
         final IndexRoutingTable indexShardRoutingTable = testRoutingTable.index(index);
@@ -151,10 +163,7 @@ private void failSomePrimaries(String index) {
             incrementVersion(index, shard); // and another time when the primary flag is set to false
         }
         RoutingAllocation.Result rerouteResult = ALLOCATION_SERVICE.applyFailedShards(this.clusterState, failedShards);
-        assertThat(rerouteResult.routingTable().version(), greaterThan(clusterState.routingTable().version()));
-        assertThat(rerouteResult.metaData().version(), greaterThan(clusterState.metaData().version()));
-        this.clusterState = ClusterState.builder(clusterState).routingResult(rerouteResult).build();
-        this.testRoutingTable = rerouteResult.routingTable();
+        applyRerouteResult(rerouteResult);
     }
 
     private IndexMetaData.Builder createIndexMetaData(String indexName) {