From 24a841a07567f781a2d5f7cd6e2d5b7dcec6c0fc Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 29 Jan 2016 13:57:18 -0500 Subject: [PATCH] ShardId equality and hash code inconsistency This commit fixes an inconsistency between ShardId#equals(Object), ShardId#hashCode, and ShardId#compareTo(ShardId). In particular, ShardId#equals(Object) compared only the numerical shard ID and the index name, but did not compare the index UUID; a similar situation applies to ShardId#compareTo(ShardId). However, ShardId#hashCode incorporated the indexUUID into its calculation. This can lead to situations where two ShardIds compare as equal yet have different hash codes. Closes #16319 --- .../java/org/elasticsearch/index/shard/ShardId.java | 10 +++++++--- .../org/elasticsearch/index/shard/IndexShardTests.java | 3 ++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/shard/ShardId.java b/core/src/main/java/org/elasticsearch/index/shard/ShardId.java index f021cb4c162..3dea5501c62 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/ShardId.java +++ b/core/src/main/java/org/elasticsearch/index/shard/ShardId.java @@ -76,7 +76,7 @@ public class ShardId implements Streamable, Comparable { if (this == o) return true; if (o == null) return false; ShardId shardId1 = (ShardId) o; - return shardId == shardId1.shardId && index.getName().equals(shardId1.index.getName()); + return shardId == shardId1.shardId && index.equals(shardId1.index); } @Override @@ -112,8 +112,12 @@ public class ShardId implements Streamable, Comparable { @Override public int compareTo(ShardId o) { if (o.getId() == shardId) { - return index.getName().compareTo(o.getIndex().getName()); + int compare = index.getName().compareTo(o.getIndex().getName()); + if (compare != 0) { + return compare; + } + return index.getUUID().compareTo(o.getIndex().getUUID()); } return Integer.compare(shardId, o.getId()); } -} \ No newline at end of file +} diff --git a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index ca0069e4eda..9a4e6a814a3 100644 --- a/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/core/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -68,6 +68,7 @@ import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.env.ShardLock; +import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexService; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.NodeServicesProvider; @@ -250,7 +251,7 @@ public class IndexShardTests extends ESSingleNodeTestCase { ShardStateMetaData shardStateMetaData = load(logger, env.availableShardPaths(shard.shardId)); assertEquals(shardStateMetaData, getShardStateMetadata(shard)); - routing = TestShardRouting.newShardRouting(shard.shardId.getIndexName(), shard.shardId.id(), routing.currentNodeId(), null, routing.primary(), ShardRoutingState.INITIALIZING, shard.shardRouting.allocationId(), shard.shardRouting.version() + 1); + routing = TestShardRouting.newShardRouting(shard.shardId.getIndex(), shard.shardId.id(), routing.currentNodeId(), null, routing.primary(), ShardRoutingState.INITIALIZING, shard.shardRouting.allocationId(), shard.shardRouting.version() + 1); shard.updateRoutingEntry(routing, true); shard.deleteShardState();