From 8dca0fece6007af7e2bbb397fc36992ac67373d6 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Fri, 9 Sep 2022 16:33:49 +0200 Subject: [PATCH 1/3] Restict the priority of ShardSnapshotTask to the same snapshot only --- .../blobstore/ShardSnapshotTaskRunner.java | 12 ++++---- .../ShardSnapshotTaskRunnerTests.java | 30 +++++++++++-------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunner.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunner.java index 8f11a5a3b830e..7c0a2ed9c65ab 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunner.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunner.java @@ -15,6 +15,7 @@ import org.elasticsearch.repositories.SnapshotShardContext; import java.io.IOException; +import java.util.Comparator; import java.util.concurrent.Executor; import java.util.function.Consumer; import java.util.function.Supplier; @@ -32,6 +33,11 @@ public class ShardSnapshotTaskRunner { private final CheckedBiConsumer fileSnapshotter; abstract static class SnapshotTask implements Comparable, Runnable { + + private static final Comparator COMPARATOR = Comparator.comparingLong( + (SnapshotTask t) -> t.context().snapshotStartTime() + ).thenComparing(t -> t.context().snapshotId().getUUID()).thenComparingInt(SnapshotTask::priority); + protected final SnapshotShardContext context; SnapshotTask(SnapshotShardContext context) { @@ -46,11 +52,7 @@ public SnapshotShardContext context() { @Override public final int compareTo(SnapshotTask other) { - int res = Integer.compare(priority(), other.priority()); - if (res != 0) { - return res; - } - return Long.compare(context.snapshotStartTime(), other.context.snapshotStartTime()); + return COMPARATOR.compare(this, other); } } diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunnerTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunnerTests.java index 2cbbf0f382ba8..5cfd872a7b1b5 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunnerTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/ShardSnapshotTaskRunnerTests.java @@ -164,34 +164,40 @@ public void testShardSnapshotTaskRunner() throws Exception { public void testCompareToShardSnapshotTask() { ShardSnapshotTaskRunner workers = new ShardSnapshotTaskRunner(1, executor, context -> {}, (context, fileInfo) -> {}); - SnapshotId s1 = new SnapshotId("s1", UUIDs.randomBase64UUID()); - SnapshotId s2 = new SnapshotId("s2", UUIDs.randomBase64UUID()); - SnapshotId s3 = new SnapshotId("s3", UUIDs.randomBase64UUID()); + SnapshotId s1 = new SnapshotId("s1", "s1-uuid"); + SnapshotId s2 = new SnapshotId("s2", "s2-uuid"); + SnapshotId s3 = new SnapshotId("s3", "s3-uuid"); ActionListener listener = ActionListener.noop(); final long s1StartTime = threadPool.absoluteTimeInMillis(); final long s2StartTime = s1StartTime + randomLongBetween(1, 1000); SnapshotShardContext s1Context = dummyContext(s1, s1StartTime); SnapshotShardContext s2Context = dummyContext(s2, s2StartTime); SnapshotShardContext s3Context = dummyContext(s3, s2StartTime); - // Two tasks with the same start time and of the same type have the same priority - assertThat(workers.new ShardSnapshotTask(s2Context).compareTo(workers.new ShardSnapshotTask(s3Context)), equalTo(0)); - // Shard snapshot task always has a higher priority over file snapshot + // Shard snapshot and file snapshot tasks for earlier snapshots have higher priority assertThat( workers.new ShardSnapshotTask(s1Context).compareTo( - workers.new FileSnapshotTask(s1Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener) + workers.new FileSnapshotTask(s2Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener) ), lessThan(0) ); assertThat( - workers.new ShardSnapshotTask(s2Context).compareTo( - workers.new FileSnapshotTask(s1Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener) + workers.new FileSnapshotTask(s1Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener).compareTo( + workers.new ShardSnapshotTask(s2Context) ), lessThan(0) ); - // File snapshots are prioritized by start time. + // Two tasks with the same start time and of the same type are ordered by snapshot UUID + assertThat(workers.new ShardSnapshotTask(s2Context).compareTo(workers.new ShardSnapshotTask(s3Context)), lessThan(0)); assertThat( - workers.new FileSnapshotTask(s1Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener).compareTo( - workers.new FileSnapshotTask(s2Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener) + workers.new FileSnapshotTask(s2Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener).compareTo( + workers.new ShardSnapshotTask(s3Context) + ), + lessThan(0) + ); + // Shard snapshot task has a higher priority over file snapshot within the same snapshot + assertThat( + workers.new ShardSnapshotTask(s1Context).compareTo( + workers.new FileSnapshotTask(s1Context, ShardSnapshotTaskRunnerTests::dummyFileInfo, listener) ), lessThan(0) ); From 0f2d7a2de405ccce6c6792807ac34663d1eb7f14 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Fri, 9 Sep 2022 18:21:31 +0200 Subject: [PATCH 2/3] Update docs/changelog/89976.yaml --- docs/changelog/89976.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/89976.yaml diff --git a/docs/changelog/89976.yaml b/docs/changelog/89976.yaml new file mode 100644 index 0000000000000..cf0407f356848 --- /dev/null +++ b/docs/changelog/89976.yaml @@ -0,0 +1,5 @@ +pr: 89976 +summary: Restrict the priority of `ShardSnapshotTask` to the same snapshot only +area: Snapshot/Restore +type: enhancement +issues: [] From af1ee556a9f1bd52817e92d913db68d3a38d1f69 Mon Sep 17 00:00:00 2001 From: Pooya Salehi Date: Mon, 12 Sep 2022 10:00:35 +0200 Subject: [PATCH 3/3] Delete docs/changelog/89976.yaml --- docs/changelog/89976.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/89976.yaml diff --git a/docs/changelog/89976.yaml b/docs/changelog/89976.yaml deleted file mode 100644 index cf0407f356848..0000000000000 --- a/docs/changelog/89976.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 89976 -summary: Restrict the priority of `ShardSnapshotTask` to the same snapshot only -area: Snapshot/Restore -type: enhancement -issues: []