diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java b/core/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java index e1b499fc606..9c0af60a16f 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java @@ -608,7 +608,12 @@ public final class ShardRouting implements Streamable, ToXContent { return false; } ShardRouting that = (ShardRouting) o; - // TODO: add version + unassigned info check. see #12387 + if (version != that.version) { + return false; + } + if (unassignedInfo != null ? !unassignedInfo.equals(that.unassignedInfo) : that.unassignedInfo != null) { + return false; + } return equalsIgnoringMetaData(that); } @@ -626,8 +631,10 @@ public final class ShardRouting implements Streamable, ToXContent { result = 31 * result + (relocatingNodeId != null ? relocatingNodeId.hashCode() : 0); result = 31 * result + (primary ? 1 : 0); result = 31 * result + (state != null ? state.hashCode() : 0); + result = 31 * result + (int) (version ^ (version >>> 32)); result = 31 * result + (restoreSource != null ? restoreSource.hashCode() : 0); result = 31 * result + (allocationId != null ? allocationId.hashCode() : 0); + result = 31 * result + (unassignedInfo != null ? unassignedInfo.hashCode() : 0); return hashCode = result; } diff --git a/core/src/main/java/org/elasticsearch/cluster/routing/UnassignedInfo.java b/core/src/main/java/org/elasticsearch/cluster/routing/UnassignedInfo.java index 1c602491f26..e6dc7184cc3 100644 --- a/core/src/main/java/org/elasticsearch/cluster/routing/UnassignedInfo.java +++ b/core/src/main/java/org/elasticsearch/cluster/routing/UnassignedInfo.java @@ -293,4 +293,27 @@ public class UnassignedInfo implements ToXContent, Writeable { builder.endObject(); return builder; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + UnassignedInfo that = (UnassignedInfo) o; + + if (timestamp != that.timestamp) return false; + if (reason != that.reason) return false; + if (message != null ? !message.equals(that.message) : that.message != null) return false; + return !(failure != null ? !failure.equals(that.failure) : that.failure != null); + + } + + @Override + public int hashCode() { + int result = reason != null ? reason.hashCode() : 0; + result = 31 * result + (int) (timestamp ^ (timestamp >>> 32)); + result = 31 * result + (message != null ? message.hashCode() : 0); + result = 31 * result + (failure != null ? failure.hashCode() : 0); + return result; + } } diff --git a/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java b/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java index a33730b8342..edd6254b511 100644 --- a/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java +++ b/core/src/test/java/org/elasticsearch/cluster/ClusterStateDiffIT.java @@ -47,8 +47,10 @@ import org.junit.Test; import java.util.List; import static org.elasticsearch.cluster.metadata.AliasMetaData.newAliasMetaDataBuilder; +import static org.elasticsearch.cluster.routing.RandomShardRoutingMutator.randomChange; +import static org.elasticsearch.cluster.routing.RandomShardRoutingMutator.randomReason; import static org.elasticsearch.test.XContentTestUtils.convertToMap; -import static org.elasticsearch.test.XContentTestUtils.mapsEqualIgnoringArrayOrder; +import static org.elasticsearch.test.XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder; import static org.elasticsearch.test.VersionUtils.randomVersion; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -151,7 +153,7 @@ public class ClusterStateDiffIT extends ESIntegTestCase { assertThat(clusterStateFromDiffs.metaData().equalsAliases(clusterState.metaData()), is(true)); // JSON Serialization test - make sure that both states produce similar JSON - assertThat(mapsEqualIgnoringArrayOrder(convertToMap(clusterStateFromDiffs), convertToMap(clusterState)), equalTo(true)); + assertNull(differenceBetweenMapsIgnoringArrayOrder(convertToMap(clusterStateFromDiffs), convertToMap(clusterState))); // Smoke test - we cannot compare bytes to bytes because some elements might get serialized in different order // however, serialized size should remain the same @@ -200,7 +202,7 @@ public class ClusterStateDiffIT extends ESIntegTestCase { if (randomBoolean()) { builder.remove(index); } else { - builder.add(randomIndexRoutingTable(index, clusterState.nodes().nodes().keys().toArray(String.class))); + builder.add(randomChangeToIndexRoutingTable(clusterState.routingTable().indicesRouting().get(index), clusterState.nodes().nodes().keys().toArray(String.class))); } } } @@ -222,14 +224,34 @@ public class ClusterStateDiffIT extends ESIntegTestCase { IndexShardRoutingTable.Builder indexShard = new IndexShardRoutingTable.Builder(new ShardId(index, i)); int replicaCount = randomIntBetween(1, 10); for (int j = 0; j < replicaCount; j++) { + UnassignedInfo unassignedInfo = null; + if (randomInt(5) == 1) { + unassignedInfo = new UnassignedInfo(randomReason(), randomAsciiOfLength(10)); + } indexShard.addShard( - TestShardRouting.newShardRouting(index, i, randomFrom(nodeIds), null, null, j == 0, ShardRoutingState.fromValue((byte) randomIntBetween(2, 4)), 1)); + TestShardRouting.newShardRouting(index, i, randomFrom(nodeIds), null, null, j == 0, + ShardRoutingState.fromValue((byte) randomIntBetween(2, 4)), 1, unassignedInfo)); } builder.addIndexShard(indexShard.build()); } return builder.build(); } + /** + * Randomly updates index routing table in the cluster state + */ + private IndexRoutingTable randomChangeToIndexRoutingTable(IndexRoutingTable original, String[] nodes) { + IndexRoutingTable.Builder builder = IndexRoutingTable.builder(original.getIndex()); + for (ObjectCursor indexShardRoutingTable : original.shards().values()) { + for (ShardRouting shardRouting : indexShardRoutingTable.value.shards()) { + final ShardRouting newShardRouting = new ShardRouting(shardRouting); + randomChange(newShardRouting, nodes); + builder.addShard(indexShardRoutingTable.value, newShardRouting); + } + } + return builder.build(); + } + /** * Randomly creates or removes cluster blocks */ diff --git a/core/src/test/java/org/elasticsearch/cluster/routing/RandomShardRoutingMutator.java b/core/src/test/java/org/elasticsearch/cluster/routing/RandomShardRoutingMutator.java new file mode 100644 index 00000000000..4ca849d8ae4 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/routing/RandomShardRoutingMutator.java @@ -0,0 +1,88 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cluster.routing; + +import static org.elasticsearch.test.ESTestCase.randomAsciiOfLength; +import static org.elasticsearch.test.ESTestCase.randomFrom; +import static org.elasticsearch.test.ESTestCase.randomInt; + +/** + * Utility class the makes random modifications to ShardRouting + */ +public final class RandomShardRoutingMutator { + private RandomShardRoutingMutator() { + + } + + public static void randomChange(ShardRouting shardRouting, String[] nodes) { + switch (randomInt(3)) { + case 0: + if (shardRouting.unassigned() == false) { + shardRouting.moveToUnassigned(new UnassignedInfo(randomReason(), randomAsciiOfLength(10))); + } else if (shardRouting.unassignedInfo() != null) { + shardRouting.updateUnassignedInfo(new UnassignedInfo(randomReason(), randomAsciiOfLength(10))); + } + break; + case 1: + if (shardRouting.unassigned()) { + shardRouting.initialize(randomFrom(nodes)); + } + break; + case 2: + if (shardRouting.primary()) { + shardRouting.moveFromPrimary(); + } else { + shardRouting.moveToPrimary(); + } + break; + case 3: + if (shardRouting.initializing()) { + shardRouting.moveToStarted(); + } + break; + } + } + + + public static UnassignedInfo.Reason randomReason() { + switch (randomInt(9)) { + case 0: + return UnassignedInfo.Reason.INDEX_CREATED; + case 1: + return UnassignedInfo.Reason.CLUSTER_RECOVERED; + case 2: + return UnassignedInfo.Reason.INDEX_REOPENED; + case 3: + return UnassignedInfo.Reason.DANGLING_INDEX_IMPORTED; + case 4: + return UnassignedInfo.Reason.NEW_INDEX_RESTORED; + case 5: + return UnassignedInfo.Reason.EXISTING_INDEX_RESTORED; + case 6: + return UnassignedInfo.Reason.REPLICA_ADDED; + case 7: + return UnassignedInfo.Reason.ALLOCATION_FAILED; + case 8: + return UnassignedInfo.Reason.NODE_LEFT; + default: + return UnassignedInfo.Reason.REROUTE_CANCELLED; + } + } +} diff --git a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java index 0784d2a75f3..8d4b7c4cb51 100644 --- a/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/core/src/test/java/org/elasticsearch/test/ESIntegTestCase.java @@ -67,7 +67,6 @@ import org.elasticsearch.client.Client; import org.elasticsearch.client.Requests; import org.elasticsearch.cluster.ClusterService; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MappingMetaData; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.cluster.routing.IndexRoutingTable; @@ -140,7 +139,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF import static org.elasticsearch.common.settings.Settings.settingsBuilder; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.test.XContentTestUtils.convertToMap; -import static org.elasticsearch.test.XContentTestUtils.mapsEqualIgnoringArrayOrder; +import static org.elasticsearch.test.XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*; import static org.hamcrest.Matchers.*; @@ -1072,7 +1071,7 @@ public abstract class ESIntegTestCase extends ESTestCase { // but we can compare serialization sizes - they should be the same assertEquals("clusterstate size does not match", masterClusterStateSize, localClusterStateSize); // Compare JSON serialization - assertTrue("clusterstate JSON serialization does not match", mapsEqualIgnoringArrayOrder(masterStateMap, localStateMap)); + assertNull("clusterstate JSON serialization does not match", differenceBetweenMapsIgnoringArrayOrder(masterStateMap, localStateMap)); } catch (AssertionError error) { logger.error("Cluster state from master:\n{}\nLocal cluster state:\n{}", masterClusterState.toString(), localClusterState.toString()); throw error; diff --git a/core/src/test/java/org/elasticsearch/test/XContentTestUtils.java b/core/src/test/java/org/elasticsearch/test/XContentTestUtils.java index 917c66a452b..647930398b6 100644 --- a/core/src/test/java/org/elasticsearch/test/XContentTestUtils.java +++ b/core/src/test/java/org/elasticsearch/test/XContentTestUtils.java @@ -46,58 +46,80 @@ public final class XContentTestUtils { /** - * Compares to maps generated from XContentObjects. The order of elements in arrays is ignored + * Compares to maps generated from XContentObjects. The order of elements in arrays is ignored. + * + * @return null if maps are equal or path to the element where the difference was found */ - public static boolean mapsEqualIgnoringArrayOrder(Map first, Map second) { + public static String differenceBetweenMapsIgnoringArrayOrder(Map first, Map second) { + return differenceBetweenMapsIgnoringArrayOrder("", first, second); + } + + private static String differenceBetweenMapsIgnoringArrayOrder(String path, Map first, Map second) { if (first.size() != second.size()) { - return false; + return path + ": sizes of the maps don't match: " + first.size() + " != " + second.size(); } for (String key : first.keySet()) { - if (objectsEqualIgnoringArrayOrder(first.get(key), second.get(key)) == false) { - return false; + String reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/" + key, first.get(key), second.get(key)); + if (reason != null) { + return reason; } } - return true; + return null; } @SuppressWarnings("unchecked") - private static boolean objectsEqualIgnoringArrayOrder(Object first, Object second) { - if (first == null ) { - return second == null; + private static String differenceBetweenObjectsIgnoringArrayOrder(String path, Object first, Object second) { + if (first == null) { + if (second == null) { + return null; + } else { + return path + ": first element is null, the second element is not null"; + } } else if (first instanceof List) { - if (second instanceof List) { + if (second instanceof List) { List secondList = Lists.newArrayList((List) second); List firstList = (List) first; if (firstList.size() == secondList.size()) { + String reason = path + ": no matches found"; for (Object firstObj : firstList) { boolean found = false; for (Object secondObj : secondList) { - if (objectsEqualIgnoringArrayOrder(firstObj, secondObj)) { + reason = differenceBetweenObjectsIgnoringArrayOrder(path + "/*", firstObj, secondObj); + if (reason == null) { secondList.remove(secondObj); found = true; break; } } if (found == false) { - return false; + return reason; } } - return secondList.isEmpty(); + if (secondList.isEmpty()) { + return null; + } else { + return path + ": the second list is not empty"; + } } else { - return false; + return path + ": sizes of the arrays don't match: " + firstList.size() + " != " + secondList.size(); } } else { - return false; + return path + ": the second element is not an array"; } } else if (first instanceof Map) { if (second instanceof Map) { - return mapsEqualIgnoringArrayOrder((Map) first, (Map) second); + return differenceBetweenMapsIgnoringArrayOrder(path, (Map) first, (Map) second); } else { - return false; + return path + ": the second element is not a map"; } } else { - return first.equals(second); + if (first.equals(second)) { + return null; + } else { + return path + ": the elements don't match: [" + first + "] != [" + second + "]"; + } + } }