From 7e68989dae13a815f0fba2b8b6deecaa72c4b47d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 10 Jan 2020 11:49:52 +0100 Subject: [PATCH] Fix Snapshot Shard Status Request Deduplication (#50788) (#50840) * Fix Snapshot Shard Status Request Deduplication The request deduplication didn't actually work for these requests since they had no `equals` and `hashCode` so the deduplicator wouldn't actually recognize equal requests. --- .../snapshots/SnapshotShardsService.java | 24 +++++++-- .../snapshots/SnapshotShardsServiceTests.java | 53 +++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/snapshots/SnapshotShardsServiceTests.java diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java index bf429c52936..4b205a55370 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardsService.java @@ -77,6 +77,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Function; import java.util.stream.Collectors; @@ -391,9 +392,9 @@ public class SnapshotShardsService extends AbstractLifecycleComponent implements * Internal request that is used to send changes in snapshot status to master */ public static class UpdateIndexShardSnapshotStatusRequest extends MasterNodeRequest { - private Snapshot snapshot; - private ShardId shardId; - private ShardSnapshotStatus status; + private final Snapshot snapshot; + private final ShardId shardId; + private final ShardSnapshotStatus status; public UpdateIndexShardSnapshotStatusRequest(StreamInput in) throws IOException { super(in); @@ -439,6 +440,23 @@ public class SnapshotShardsService extends AbstractLifecycleComponent implements public String toString() { return snapshot + ", shardId [" + shardId + "], status [" + status.state() + "]"; } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + final UpdateIndexShardSnapshotStatusRequest that = (UpdateIndexShardSnapshotStatusRequest) o; + return snapshot.equals(that.snapshot) && shardId.equals(that.shardId) && status.equals(that.status); + } + + @Override + public int hashCode() { + return Objects.hash(snapshot, shardId, status); + } } /** Notify the master node that the given shard has been successfully snapshotted **/ diff --git a/server/src/test/java/org/elasticsearch/snapshots/SnapshotShardsServiceTests.java b/server/src/test/java/org/elasticsearch/snapshots/SnapshotShardsServiceTests.java new file mode 100644 index 00000000000..f1b4928c0e2 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/snapshots/SnapshotShardsServiceTests.java @@ -0,0 +1,53 @@ +/* + * 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.snapshots; + +import org.elasticsearch.cluster.SnapshotsInProgress; +import org.elasticsearch.common.UUIDs; +import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.EqualsHashCodeTestUtils; + +public class SnapshotShardsServiceTests extends ESTestCase { + + public void testEqualsAndHashcodeUpdateIndexShardSnapshotStatusRequest() { + EqualsHashCodeTestUtils.checkEqualsAndHashCode( + new SnapshotShardsService.UpdateIndexShardSnapshotStatusRequest( + new Snapshot(randomAlphaOfLength(10), + new SnapshotId(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random()))), + new ShardId(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random()), randomInt(5)), + new SnapshotsInProgress.ShardSnapshotStatus(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random()))), + request -> + new SnapshotShardsService.UpdateIndexShardSnapshotStatusRequest(request.snapshot(), request.shardId(), request.status()), + request -> { + final boolean mutateSnapshot = randomBoolean(); + final boolean mutateShardId = randomBoolean(); + final boolean mutateStatus = (mutateSnapshot || mutateShardId) == false || randomBoolean(); + return new SnapshotShardsService.UpdateIndexShardSnapshotStatusRequest( + mutateSnapshot ? new Snapshot(randomAlphaOfLength(10), + new SnapshotId(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random()))) : request.snapshot(), + mutateShardId ? + new ShardId(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random()), randomInt(5)) : request.shardId(), + mutateStatus ? new SnapshotsInProgress.ShardSnapshotStatus(randomAlphaOfLength(10), UUIDs.randomBase64UUID(random())) + : request.status() + ); + }); + } + +}