Restrict the priority of ShardSnapshotTask to the same snapshot only

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<SnapshotShardContext, FileInfo, IOException> fileSnapshotter;
 
     abstract static class SnapshotTask implements Comparable<SnapshotTask>, Runnable {
+
+        private static final Comparator<SnapshotTask> 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);
         }
     }
 

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<Void> 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)
         );