浏览代码

Cleanup some code in recovery codebase (#94360)

Just some random and obvious cleanups I found going through this codebase again.
Armin Braun 2 年之前
父节点
当前提交
c5fb9e3e58
共有 22 个文件被更改,包括 116 次插入251 次删除
  1. 2 2
      server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
  2. 1 14
      server/src/main/java/org/elasticsearch/indices/IndicesService.java
  3. 2 7
      server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java
  4. 1 11
      server/src/main/java/org/elasticsearch/indices/recovery/MultiChunkTransfer.java
  5. 8 11
      server/src/main/java/org/elasticsearch/indices/recovery/MultiFileWriter.java
  6. 5 21
      server/src/main/java/org/elasticsearch/indices/recovery/PeerRecoverySourceService.java
  7. 7 9
      server/src/main/java/org/elasticsearch/indices/recovery/PeerRecoveryTargetService.java
  8. 2 2
      server/src/main/java/org/elasticsearch/indices/recovery/RecoveryFilesInfoRequest.java
  9. 3 3
      server/src/main/java/org/elasticsearch/indices/recovery/RecoveryHandoffPrimaryContextRequest.java
  10. 48 106
      server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java
  11. 1 6
      server/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java
  12. 1 1
      server/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java
  13. 9 9
      server/src/main/java/org/elasticsearch/indices/recovery/StartRecoveryRequest.java
  14. 4 23
      server/src/main/java/org/elasticsearch/indices/recovery/plan/ShardRecoveryPlan.java
  15. 0 4
      server/src/main/java/org/elasticsearch/indices/recovery/plan/ShardSnapshot.java
  16. 4 4
      server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java
  17. 11 11
      server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java
  18. 1 1
      server/src/test/java/org/elasticsearch/indices/recovery/RecoveryTests.java
  19. 2 2
      server/src/test/java/org/elasticsearch/recovery/RecoveriesCollectionTests.java
  20. 1 1
      test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java
  21. 1 1
      x-pack/plugin/snapshot-based-recoveries/src/main/java/org/elasticsearch/xpack/snapshotbasedrecoveries/recovery/plan/SnapshotsRecoveryPlannerService.java
  22. 2 2
      x-pack/plugin/snapshot-based-recoveries/src/test/java/org/elasticsearch/xpack/snapshotbasedrecoveries/recovery/plan/SnapshotsRecoveryPlannerServiceTests.java

+ 2 - 2
server/src/main/java/org/elasticsearch/index/shard/IndexShard.java

@@ -3061,7 +3061,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
                     recoveryTargetService.startRecovery(this, recoveryState.getSourceNode(), recoveryListener);
                 } catch (Exception e) {
                     failShard("corrupted preexisting index", e);
-                    recoveryListener.onRecoveryFailure(recoveryState, new RecoveryFailedException(recoveryState, null, e), true);
+                    recoveryListener.onRecoveryFailure(new RecoveryFailedException(recoveryState, null, e), true);
                 }
                 break;
             case SNAPSHOT:
@@ -3144,7 +3144,7 @@ public class IndexShard extends AbstractIndexShardComponent implements IndicesCl
             if (r) {
                 recoveryListener.onRecoveryDone(recoveryState, getTimestampRange());
             }
-        }, e -> recoveryListener.onRecoveryFailure(recoveryState, new RecoveryFailedException(recoveryState, null, e), true)), action));
+        }, e -> recoveryListener.onRecoveryFailure(new RecoveryFailedException(recoveryState, null, e), true)), action));
     }
 
     /**

+ 1 - 14
server/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -17,7 +17,6 @@ import org.apache.lucene.util.CollectionUtil;
 import org.apache.lucene.util.RamUsageEstimator;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.ResourceAlreadyExistsException;
-import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.indices.stats.CommonStats;
 import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags;
 import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags.Flag;
@@ -1227,12 +1226,7 @@ public class IndicesService extends AbstractLifecycleComponent
 
     private void addPendingDelete(Index index, PendingDelete pendingDelete) {
         synchronized (pendingDeletes) {
-            List<PendingDelete> list = pendingDeletes.get(index);
-            if (list == null) {
-                list = new ArrayList<>();
-                pendingDeletes.put(index, list);
-            }
-            list.add(pendingDelete);
+            pendingDeletes.computeIfAbsent(index, k -> new ArrayList<>()).add(pendingDelete);
             numUncompletedDeletes.incrementAndGet();
         }
     }
@@ -1728,13 +1722,6 @@ public class IndicesService extends AbstractLifecycleComponent
         return mapperRegistry.getFieldFilter();
     }
 
-    /**
-     * Returns the registered metadata field names for the provided compatible {@link Version}.
-     */
-    public Set<String> getMetadataFields(Version version) {
-        return mapperRegistry.getMetadataMapperParsers(version).keySet();
-    }
-
     private void setIdFieldDataEnabled(boolean value) {
         this.idFieldDataEnabled = value;
     }

+ 2 - 7
server/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java

@@ -268,18 +268,13 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent imple
                 new GlobalCheckpointSyncAction.Request(shardId),
                 ActionListener.wrap(r -> {}, e -> {
                     if (ExceptionsHelper.unwrap(e, AlreadyClosedException.class, IndexShardClosedException.class) == null) {
-                        getLogger().info(() -> format("%s global checkpoint sync failed", shardId), e);
+                        logger.info(() -> format("%s global checkpoint sync failed", shardId), e);
                     }
                 })
             );
         }
     }
 
-    // overrideable by tests
-    static Logger getLogger() {
-        return logger;
-    }
-
     /**
      * Deletes indices (with shard data).
      *
@@ -709,7 +704,7 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent imple
         }
 
         @Override
-        public void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure) {
+        public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) {
             handleRecoveryFailure(shardRouting, sendShardFailure, e);
         }
     }

+ 1 - 11
server/src/main/java/org/elasticsearch/indices/recovery/MultiChunkTransfer.java

@@ -185,17 +185,7 @@ public abstract class MultiChunkTransfer<Source, Request extends MultiChunkTrans
 
     protected abstract void handleError(Source resource, Exception e) throws Exception;
 
-    private static class FileChunkResponseItem<Source> {
-        final long requestSeqId;
-        final Source source;
-        final Exception failure;
-
-        FileChunkResponseItem(long requestSeqId, Source source, Exception failure) {
-            this.requestSeqId = requestSeqId;
-            this.source = source;
-            this.failure = failure;
-        }
-    }
+    private record FileChunkResponseItem<Source> (long requestSeqId, Source source, Exception failure) {}
 
     public interface ChunkRequest {
         /**

+ 8 - 11
server/src/main/java/org/elasticsearch/indices/recovery/MultiFileWriter.java

@@ -95,7 +95,7 @@ public class MultiFileWriter extends AbstractRefCounted implements Releasable {
         tempFileNames.put(tempFileName, fileName);
 
         incRef();
-        try (IndexOutput indexOutput = createIndexOutput(tempFileName, fileMetadata, IOContext.DEFAULT)) {
+        try (IndexOutput indexOutput = createIndexOutput(tempFileName, fileMetadata)) {
             int bufferSize = Math.toIntExact(Math.min(readSnapshotFileBufferSize, fileMetadata.length()));
             byte[] buffer = new byte[bufferSize];
             int length;
@@ -133,10 +133,10 @@ public class MultiFileWriter extends AbstractRefCounted implements Releasable {
         }
     }
 
-    private IndexOutput createIndexOutput(String tempFileName, StoreFileMetadata fileMetadata, IOContext context) throws IOException {
+    private IndexOutput createIndexOutput(String tempFileName, StoreFileMetadata fileMetadata) throws IOException {
         return verifyOutput
-            ? store.createVerifyingOutput(tempFileName, fileMetadata, context)
-            : store.directory().createOutput(tempFileName, context);
+            ? store.createVerifyingOutput(tempFileName, fileMetadata, IOContext.DEFAULT)
+            : store.directory().createOutput(tempFileName, IOContext.DEFAULT);
     }
 
     /** Get a temporary name for the provided file name. */
@@ -245,13 +245,10 @@ public class MultiFileWriter extends AbstractRefCounted implements Releasable {
         store.renameTempFilesSafe(tempFileNames);
     }
 
-    private static final class FileChunk implements Releasable {
-        final StoreFileMetadata md;
-        final ReleasableBytesReference content;
-        final long position;
-        final boolean lastChunk;
-
-        FileChunk(StoreFileMetadata md, ReleasableBytesReference content, long position, boolean lastChunk) {
+    private record FileChunk(StoreFileMetadata md, ReleasableBytesReference content, long position, boolean lastChunk)
+        implements
+            Releasable {
+        private FileChunk(StoreFileMetadata md, ReleasableBytesReference content, long position, boolean lastChunk) {
             this.md = md;
             this.content = content.retain();
             this.position = position;

+ 5 - 21
server/src/main/java/org/elasticsearch/indices/recovery/PeerRecoverySourceService.java

@@ -34,8 +34,6 @@ import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.indices.recovery.plan.RecoveryPlannerService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.threadpool.ThreadPool;
-import org.elasticsearch.transport.TransportChannel;
-import org.elasticsearch.transport.TransportRequestHandler;
 import org.elasticsearch.transport.TransportService;
 
 import java.util.ArrayList;
@@ -83,7 +81,7 @@ public class PeerRecoverySourceService extends AbstractLifecycleComponent implem
             Actions.START_RECOVERY,
             ThreadPool.Names.GENERIC,
             StartRecoveryRequest::new,
-            new StartRecoveryTransportRequestHandler()
+            (request, channel, task) -> recover(request, task, new ChannelActionListener<>(channel))
         );
         // When the target node's START_RECOVERY request has failed due to a network disconnection, it will
         // send a REESTABLISH_RECOVERY. This attempts to reconnect to an existing recovery process taking
@@ -93,7 +91,7 @@ public class PeerRecoverySourceService extends AbstractLifecycleComponent implem
             Actions.REESTABLISH_RECOVERY,
             ThreadPool.Names.GENERIC,
             ReestablishRecoveryRequest::new,
-            new ReestablishRecoveryTransportRequestHandler()
+            (request, channel, task) -> reestablish(request, new ChannelActionListener<>(channel))
         );
     }
 
@@ -120,7 +118,7 @@ public class PeerRecoverySourceService extends AbstractLifecycleComponent implem
     @Override
     public void beforeIndexShardClosed(ShardId shardId, @Nullable IndexShard indexShard, Settings indexSettings) {
         if (indexShard != null) {
-            ongoingRecoveries.cancel(indexShard, "shard is closed");
+            ongoingRecoveries.cancel(indexShard);
         }
     }
 
@@ -176,20 +174,6 @@ public class PeerRecoverySourceService extends AbstractLifecycleComponent implem
         ongoingRecoveries.reestablishRecovery(request, shard, listener);
     }
 
-    class StartRecoveryTransportRequestHandler implements TransportRequestHandler<StartRecoveryRequest> {
-        @Override
-        public void messageReceived(final StartRecoveryRequest request, final TransportChannel channel, Task task) throws Exception {
-            recover(request, task, new ChannelActionListener<>(channel));
-        }
-    }
-
-    class ReestablishRecoveryTransportRequestHandler implements TransportRequestHandler<ReestablishRecoveryRequest> {
-        @Override
-        public void messageReceived(final ReestablishRecoveryRequest request, final TransportChannel channel, Task task) throws Exception {
-            reestablish(request, new ChannelActionListener<>(channel));
-        }
-    }
-
     // exposed for testing
     final int numberOfOngoingRecoveries() {
         return ongoingRecoveries.ongoingRecoveries.size();
@@ -266,13 +250,13 @@ public class PeerRecoverySourceService extends AbstractLifecycleComponent implem
             }
         }
 
-        synchronized void cancel(IndexShard shard, String reason) {
+        synchronized void cancel(IndexShard shard) {
             final ShardRecoveryContext shardRecoveryContext = ongoingRecoveries.get(shard);
             if (shardRecoveryContext != null) {
                 final List<Exception> failures = new ArrayList<>();
                 for (RecoverySourceHandler handlers : shardRecoveryContext.recoveryHandlers.keySet()) {
                     try {
-                        handlers.cancel(reason);
+                        handlers.cancel("shard is closed");
                     } catch (Exception ex) {
                         failures.add(ex);
                     } finally {

+ 7 - 9
server/src/main/java/org/elasticsearch/indices/recovery/PeerRecoveryTargetService.java

@@ -397,7 +397,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     public interface RecoveryListener {
         void onRecoveryDone(RecoveryState state, ShardLongFieldRange timestampMillisFieldRange);
 
-        void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure);
+        void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure);
     }
 
     class PrepareForTranslogOperationsRequestHandler implements TransportRequestHandler<RecoveryPrepareForTranslogOperationsRequest> {
@@ -418,7 +418,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     class FinalizeRecoveryRequestHandler implements TransportRequestHandler<RecoveryFinalizeRecoveryRequest> {
 
         @Override
-        public void messageReceived(RecoveryFinalizeRecoveryRequest request, TransportChannel channel, Task task) throws Exception {
+        public void messageReceived(RecoveryFinalizeRecoveryRequest request, TransportChannel channel, Task task) {
             try (RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.recoveryId(), request.shardId())) {
                 final ActionListener<Void> listener = createOrFinishListener(recoveryRef, channel, request);
                 if (listener == null) {
@@ -433,8 +433,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     class HandoffPrimaryContextRequestHandler implements TransportRequestHandler<RecoveryHandoffPrimaryContextRequest> {
 
         @Override
-        public void messageReceived(final RecoveryHandoffPrimaryContextRequest request, final TransportChannel channel, Task task)
-            throws Exception {
+        public void messageReceived(final RecoveryHandoffPrimaryContextRequest request, final TransportChannel channel, Task task) {
             final RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.recoveryId(), request.shardId());
             boolean success = false;
             try {
@@ -459,8 +458,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     class TranslogOperationsRequestHandler implements TransportRequestHandler<RecoveryTranslogOperationsRequest> {
 
         @Override
-        public void messageReceived(final RecoveryTranslogOperationsRequest request, final TransportChannel channel, Task task)
-            throws IOException {
+        public void messageReceived(final RecoveryTranslogOperationsRequest request, final TransportChannel channel, Task task) {
             try (RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.recoveryId(), request.shardId())) {
                 final RecoveryTarget recoveryTarget = recoveryRef.target();
                 final ActionListener<Void> listener = createOrFinishListener(
@@ -540,7 +538,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     class FilesInfoRequestHandler implements TransportRequestHandler<RecoveryFilesInfoRequest> {
 
         @Override
-        public void messageReceived(RecoveryFilesInfoRequest request, TransportChannel channel, Task task) throws Exception {
+        public void messageReceived(RecoveryFilesInfoRequest request, TransportChannel channel, Task task) {
             try (RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.recoveryId(), request.shardId())) {
                 final ActionListener<Void> listener = createOrFinishListener(recoveryRef, channel, request);
                 if (listener == null) {
@@ -563,7 +561,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
     class CleanFilesRequestHandler implements TransportRequestHandler<RecoveryCleanFilesRequest> {
 
         @Override
-        public void messageReceived(RecoveryCleanFilesRequest request, TransportChannel channel, Task task) throws Exception {
+        public void messageReceived(RecoveryCleanFilesRequest request, TransportChannel channel, Task task) {
             try (RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.recoveryId(), request.shardId())) {
                 final ActionListener<Void> listener = createOrFinishListener(recoveryRef, channel, request);
                 if (listener == null) {
@@ -631,7 +629,7 @@ public class PeerRecoveryTargetService implements IndexEventListener {
 
     class RestoreFileFromSnapshotTransportRequestHandler implements TransportRequestHandler<RecoverySnapshotFileRequest> {
         @Override
-        public void messageReceived(final RecoverySnapshotFileRequest request, TransportChannel channel, Task task) throws Exception {
+        public void messageReceived(final RecoverySnapshotFileRequest request, TransportChannel channel, Task task) {
             try (RecoveryRef recoveryRef = onGoingRecoveries.getRecoverySafe(request.getRecoveryId(), request.getShardId())) {
                 final RecoveryTarget recoveryTarget = recoveryRef.target();
                 final ActionListener<Void> listener = createOrFinishListener(recoveryRef, channel, request);

+ 2 - 2
server/src/main/java/org/elasticsearch/indices/recovery/RecoveryFilesInfoRequest.java

@@ -18,8 +18,8 @@ import java.util.List;
 
 public class RecoveryFilesInfoRequest extends RecoveryTransportRequest {
 
-    private long recoveryId;
-    private ShardId shardId;
+    private final long recoveryId;
+    private final ShardId shardId;
 
     List<String> phase1FileNames;
     List<Long> phase1FileSizes;

+ 3 - 3
server/src/main/java/org/elasticsearch/indices/recovery/RecoveryHandoffPrimaryContextRequest.java

@@ -21,9 +21,9 @@ import java.io.IOException;
  */
 class RecoveryHandoffPrimaryContextRequest extends TransportRequest {
 
-    private long recoveryId;
-    private ShardId shardId;
-    private ReplicationTracker.PrimaryContext primaryContext;
+    private final long recoveryId;
+    private final ShardId shardId;
+    private final ReplicationTracker.PrimaryContext primaryContext;
 
     /**
      * Initialize an empty request (used to serialize into when reading from a stream).

+ 48 - 106
server/src/main/java/org/elasticsearch/indices/recovery/RecoverySourceHandler.java

@@ -478,34 +478,15 @@ public class RecoverySourceHandler {
         });
     }
 
-    static final class SendFileResult {
-        final List<String> phase1FileNames;
-        final List<Long> phase1FileSizes;
-        final long totalSize;
-
-        final List<String> phase1ExistingFileNames;
-        final List<Long> phase1ExistingFileSizes;
-        final long existingTotalSize;
-
-        final TimeValue took;
-
-        SendFileResult(
-            List<String> phase1FileNames,
-            List<Long> phase1FileSizes,
-            long totalSize,
-            List<String> phase1ExistingFileNames,
-            List<Long> phase1ExistingFileSizes,
-            long existingTotalSize,
-            TimeValue took
-        ) {
-            this.phase1FileNames = phase1FileNames;
-            this.phase1FileSizes = phase1FileSizes;
-            this.totalSize = totalSize;
-            this.phase1ExistingFileNames = phase1ExistingFileNames;
-            this.phase1ExistingFileSizes = phase1ExistingFileSizes;
-            this.existingTotalSize = existingTotalSize;
-            this.took = took;
-        }
+    record SendFileResult(
+        List<String> phase1FileNames,
+        List<Long> phase1FileSizes,
+        long totalSize,
+        List<String> phase1ExistingFileNames,
+        List<Long> phase1ExistingFileSizes,
+        long existingTotalSize,
+        TimeValue took
+    ) {
 
         static final SendFileResult EMPTY = new SendFileResult(
             Collections.emptyList(),
@@ -672,7 +653,7 @@ public class RecoverySourceHandler {
                 shardRecoveryPlan.getSnapshotFilesToRecover().size(),
                 ByteSizeValue.ofBytes(
                     shardRecoveryPlan.getSnapshotFilesToRecover()
-                        .getSnapshotFiles()
+                        .snapshotFiles()
                         .stream()
                         .mapToLong(BlobStoreIndexShardSnapshot.FileInfo::length)
                         .sum()
@@ -702,32 +683,30 @@ public class RecoverySourceHandler {
             sendFileInfoStep
         );
 
-        sendFileInfoStep.whenComplete(unused -> {
-            recoverSnapshotFiles(shardRecoveryPlan, new ActionListener<>() {
-                @Override
-                public void onResponse(List<StoreFileMetadata> filesFailedToRecoverFromSnapshot) {
-                    recoverSnapshotFilesStep.onResponse(Tuple.tuple(shardRecoveryPlan, filesFailedToRecoverFromSnapshot));
-                }
+        sendFileInfoStep.whenComplete(unused -> recoverSnapshotFiles(shardRecoveryPlan, new ActionListener<>() {
+            @Override
+            public void onResponse(List<StoreFileMetadata> filesFailedToRecoverFromSnapshot) {
+                recoverSnapshotFilesStep.onResponse(Tuple.tuple(shardRecoveryPlan, filesFailedToRecoverFromSnapshot));
+            }
 
-                @Override
-                public void onFailure(Exception e) {
-                    if (shardRecoveryPlan.canRecoverSnapshotFilesFromSourceNode() == false
-                        && e instanceof CancellableThreads.ExecutionCancelledException == false) {
-                        ShardRecoveryPlan fallbackPlan = shardRecoveryPlan.getFallbackPlan();
-                        recoveryTarget.receiveFileInfo(
-                            fallbackPlan.getFilesToRecoverNames(),
-                            fallbackPlan.getFilesToRecoverSizes(),
-                            fallbackPlan.getFilesPresentInTargetNames(),
-                            fallbackPlan.getFilesPresentInTargetSizes(),
-                            fallbackPlan.getTranslogOps(),
-                            recoverSnapshotFilesStep.map(r -> Tuple.tuple(fallbackPlan, Collections.emptyList()))
-                        );
-                    } else {
-                        recoverSnapshotFilesStep.onFailure(e);
-                    }
+            @Override
+            public void onFailure(Exception e) {
+                if (shardRecoveryPlan.canRecoverSnapshotFilesFromSourceNode() == false
+                    && e instanceof CancellableThreads.ExecutionCancelledException == false) {
+                    ShardRecoveryPlan fallbackPlan = shardRecoveryPlan.getFallbackPlan();
+                    recoveryTarget.receiveFileInfo(
+                        fallbackPlan.getFilesToRecoverNames(),
+                        fallbackPlan.getFilesToRecoverSizes(),
+                        fallbackPlan.getFilesPresentInTargetNames(),
+                        fallbackPlan.getFilesPresentInTargetSizes(),
+                        fallbackPlan.getTranslogOps(),
+                        recoverSnapshotFilesStep.map(r -> Tuple.tuple(fallbackPlan, Collections.emptyList()))
+                    );
+                } else {
+                    recoverSnapshotFilesStep.onFailure(e);
                 }
-            });
-        }, listener::onFailure);
+            }
+        }), listener::onFailure);
 
         recoverSnapshotFilesStep.whenComplete(planAndFilesFailedToRecoverFromSnapshot -> {
             ShardRecoveryPlan recoveryPlan = planAndFilesFailedToRecoverFromSnapshot.v1();
@@ -747,12 +726,13 @@ public class RecoverySourceHandler {
             );
         }, listener::onFailure);
 
-        sendFilesStep.whenComplete(recoveryPlan -> {
-            createRetentionLease(
+        sendFilesStep.whenComplete(
+            recoveryPlan -> createRetentionLease(
                 recoveryPlan.getStartingSeqNo(),
                 createRetentionLeaseStep.map(retentionLease -> Tuple.tuple(recoveryPlan, retentionLease))
-            );
-        }, listener::onFailure);
+            ),
+            listener::onFailure
+        );
 
         createRetentionLeaseStep.whenComplete(recoveryPlanAndRetentionLease -> {
             final ShardRecoveryPlan recoveryPlan = recoveryPlanAndRetentionLease.v1();
@@ -821,9 +801,7 @@ public class RecoverySourceHandler {
             this.snapshotFilesToRecover = shardRecoveryPlan.getSnapshotFilesToRecover();
             this.listener = listener;
             this.countDown = new CountDown(shardRecoveryPlan.getSnapshotFilesToRecover().size());
-            this.pendingSnapshotFilesToRecover = new LinkedBlockingQueue<>(
-                shardRecoveryPlan.getSnapshotFilesToRecover().getSnapshotFiles()
-            );
+            this.pendingSnapshotFilesToRecover = new LinkedBlockingQueue<>(shardRecoveryPlan.getSnapshotFilesToRecover().snapshotFiles());
         }
 
         void start() {
@@ -876,8 +854,8 @@ public class RecoverySourceHandler {
 
                 trackOutstandingRequest(requestFuture);
                 recoveryTarget.restoreFileFromSnapshot(
-                    snapshotFilesToRecover.getRepository(),
-                    snapshotFilesToRecover.getIndexId(),
+                    snapshotFilesToRecover.repository(),
+                    snapshotFilesToRecover.indexId(),
                     snapshotFileToRecover,
                     ActionListener.runBefore(requestFuture, () -> unTrackOutstandingRequest(requestFuture))
                 );
@@ -1127,20 +1105,9 @@ public class RecoverySourceHandler {
         sender.start();
     }
 
-    private static class OperationChunkRequest implements MultiChunkTransfer.ChunkRequest {
-        final List<Translog.Operation> operations;
-        final boolean lastChunk;
-
-        OperationChunkRequest(List<Translog.Operation> operations, boolean lastChunk) {
-            this.operations = operations;
-            this.lastChunk = lastChunk;
-        }
-
-        @Override
-        public boolean lastChunk() {
-            return lastChunk;
-        }
-    }
+    private record OperationChunkRequest(List<Translog.Operation> operations, boolean lastChunk)
+        implements
+            MultiChunkTransfer.ChunkRequest {}
 
     private class OperationBatchSender extends MultiChunkTransfer<Translog.Snapshot, OperationChunkRequest> {
         private final long startingSeqNo;
@@ -1292,17 +1259,7 @@ public class RecoverySourceHandler {
         listener.onResponse(null);
     }
 
-    static final class SendSnapshotResult {
-        final long targetLocalCheckpoint;
-        final int sentOperations;
-        final TimeValue tookTime;
-
-        SendSnapshotResult(final long targetLocalCheckpoint, final int sentOperations, final TimeValue tookTime) {
-            this.targetLocalCheckpoint = targetLocalCheckpoint;
-            this.sentOperations = sentOperations;
-            this.tookTime = tookTime;
-        }
-    }
+    record SendSnapshotResult(long targetLocalCheckpoint, int sentOperations, TimeValue tookTime) {}
 
     /**
      * Cancels the recovery and interrupts all eligible threads.
@@ -1324,25 +1281,10 @@ public class RecoverySourceHandler {
             + '}';
     }
 
-    private static class FileChunk implements MultiChunkTransfer.ChunkRequest, Releasable {
-        final StoreFileMetadata md;
-        final BytesReference content;
-        final long position;
-        final boolean lastChunk;
-        final Releasable onClose;
-
-        FileChunk(StoreFileMetadata md, BytesReference content, long position, boolean lastChunk, Releasable onClose) {
-            this.md = md;
-            this.content = content;
-            this.position = position;
-            this.lastChunk = lastChunk;
-            this.onClose = onClose;
-        }
-
-        @Override
-        public boolean lastChunk() {
-            return lastChunk;
-        }
+    private record FileChunk(StoreFileMetadata md, BytesReference content, long position, boolean lastChunk, Releasable onClose)
+        implements
+            MultiChunkTransfer.ChunkRequest,
+            Releasable {
 
         @Override
         public void close() {

+ 1 - 6
server/src/main/java/org/elasticsearch/indices/recovery/RecoveryState.java

@@ -833,10 +833,6 @@ public class RecoveryState implements ToXContentFragment, Writeable {
             return fileDetails.size();
         }
 
-        public boolean isEmpty() {
-            return fileDetails.isEmpty();
-        }
-
         public void clear() {
             fileDetails.clear();
             complete = false;
@@ -990,8 +986,7 @@ public class RecoveryState implements ToXContentFragment, Writeable {
             if (total == recovered) {
                 return 100.0f;
             } else {
-                float result = 100.0f * (recovered / (float) total);
-                return result;
+                return 100.0f * (recovered / (float) total);
             }
         }
 

+ 1 - 1
server/src/main/java/org/elasticsearch/indices/recovery/RecoveryTarget.java

@@ -296,7 +296,7 @@ public class RecoveryTarget extends AbstractRefCounted implements RecoveryTarget
     }
 
     public void notifyListener(RecoveryFailedException e, boolean sendShardFailure) {
-        listener.onRecoveryFailure(state(), e, sendShardFailure);
+        listener.onRecoveryFailure(e, sendShardFailure);
     }
 
     /** mark the current recovery as done */

+ 9 - 9
server/src/main/java/org/elasticsearch/indices/recovery/StartRecoveryRequest.java

@@ -23,15 +23,15 @@ import java.io.IOException;
  */
 public class StartRecoveryRequest extends TransportRequest {
 
-    private long recoveryId;
-    private ShardId shardId;
-    private String targetAllocationId;
-    private DiscoveryNode sourceNode;
-    private DiscoveryNode targetNode;
-    private Store.MetadataSnapshot metadataSnapshot;
-    private boolean primaryRelocation;
-    private long startingSeqNo;
-    private boolean canDownloadSnapshotFiles;
+    private final long recoveryId;
+    private final ShardId shardId;
+    private final String targetAllocationId;
+    private final DiscoveryNode sourceNode;
+    private final DiscoveryNode targetNode;
+    private final Store.MetadataSnapshot metadataSnapshot;
+    private final boolean primaryRelocation;
+    private final long startingSeqNo;
+    private final boolean canDownloadSnapshotFiles;
 
     public StartRecoveryRequest(StreamInput in) throws IOException {
         super(in);

+ 4 - 23
server/src/main/java/org/elasticsearch/indices/recovery/plan/ShardRecoveryPlan.java

@@ -126,26 +126,11 @@ public class ShardRecoveryPlan {
         );
     }
 
-    public static class SnapshotFilesToRecover implements Iterable<BlobStoreIndexShardSnapshot.FileInfo> {
-        public static final SnapshotFilesToRecover EMPTY = new SnapshotFilesToRecover(null, null, emptyList());
-
-        private final IndexId indexId;
-        private final String repository;
-        private final List<BlobStoreIndexShardSnapshot.FileInfo> snapshotFiles;
-
-        public SnapshotFilesToRecover(IndexId indexId, String repository, List<BlobStoreIndexShardSnapshot.FileInfo> snapshotFiles) {
-            this.indexId = indexId;
-            this.repository = repository;
-            this.snapshotFiles = snapshotFiles;
-        }
-
-        public IndexId getIndexId() {
-            return indexId;
-        }
+    public record SnapshotFilesToRecover(IndexId indexId, String repository, List<BlobStoreIndexShardSnapshot.FileInfo> snapshotFiles)
+        implements
+            Iterable<BlobStoreIndexShardSnapshot.FileInfo> {
 
-        public String getRepository() {
-            return repository;
-        }
+        public static final SnapshotFilesToRecover EMPTY = new SnapshotFilesToRecover(null, null, emptyList());
 
         public int size() {
             return snapshotFiles.size();
@@ -155,10 +140,6 @@ public class ShardRecoveryPlan {
             return snapshotFiles.isEmpty();
         }
 
-        public List<BlobStoreIndexShardSnapshot.FileInfo> getSnapshotFiles() {
-            return snapshotFiles;
-        }
-
         @Override
         public Iterator<BlobStoreIndexShardSnapshot.FileInfo> iterator() {
             return snapshotFiles.iterator();

+ 0 - 4
server/src/main/java/org/elasticsearch/indices/recovery/plan/ShardSnapshot.java

@@ -66,10 +66,6 @@ public class ShardSnapshot {
         return shardSnapshotInfo.getIndexId();
     }
 
-    public long getStartedAt() {
-        return shardSnapshotInfo.getStartedAt();
-    }
-
     public ShardSnapshotInfo getShardSnapshotInfo() {
         return shardSnapshotInfo;
     }

+ 4 - 4
server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java

@@ -426,10 +426,10 @@ public class IndicesStore implements ClusterStateListener, Closeable {
     }
 
     private static class ShardActiveRequest extends TransportRequest {
-        protected TimeValue timeout = null;
-        private ClusterName clusterName;
-        private String indexUUID;
-        private ShardId shardId;
+        private final TimeValue timeout;
+        private final ClusterName clusterName;
+        private final String indexUUID;
+        private final ShardId shardId;
 
         ShardActiveRequest(StreamInput in) throws IOException {
             super(in);

+ 11 - 11
server/src/test/java/org/elasticsearch/indices/recovery/RecoverySourceHandlerTests.java

@@ -309,13 +309,13 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
         );
         final int expectedOps = (int) (endingSeqNo - startingSeqNo + 1);
         RecoverySourceHandler.SendSnapshotResult result = future.actionGet();
-        assertThat(result.sentOperations, equalTo(expectedOps));
+        assertThat(result.sentOperations(), equalTo(expectedOps));
         List<Translog.Operation> sortedShippedOps = shippedOps.stream().sorted(Comparator.comparing(Translog.Operation::seqNo)).toList();
         assertThat(shippedOps.size(), equalTo(expectedOps));
         for (int i = 0; i < shippedOps.size(); i++) {
             assertThat(sortedShippedOps.get(i), equalTo(operations.get(i + (int) startingSeqNo + initialNumberOfDocs)));
         }
-        assertThat(result.targetLocalCheckpoint, equalTo(checkpointOnTarget.get()));
+        assertThat(result.targetLocalCheckpoint(), equalTo(checkpointOnTarget.get()));
     }
 
     public void testSendSnapshotStopOnError() throws Exception {
@@ -450,8 +450,8 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
         );
         RecoverySourceHandler.SendSnapshotResult sendSnapshotResult = sendFuture.actionGet();
         assertTrue(received.get());
-        assertThat(sendSnapshotResult.targetLocalCheckpoint, equalTo(localCheckpoint.get()));
-        assertThat(sendSnapshotResult.sentOperations, equalTo(receivedSeqNos.size()));
+        assertThat(sendSnapshotResult.targetLocalCheckpoint(), equalTo(localCheckpoint.get()));
+        assertThat(sendSnapshotResult.sentOperations(), equalTo(receivedSeqNos.size()));
         Set<Long> sentSeqNos = new HashSet<>();
         for (Translog.Operation op : operations) {
             if (startingSeqNo <= op.seqNo() && op.seqNo() <= endingSeqNo && skipOperations.contains(op) == false) {
@@ -1186,7 +1186,7 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
             final ShardRecoveryPlan shardRecoveryPlan = createShardRecoveryPlan(store, randomIntBetween(10, 20), randomIntBetween(10, 20));
 
             final ShardRecoveryPlan.SnapshotFilesToRecover snapshotFilesToRecover = shardRecoveryPlan.getSnapshotFilesToRecover();
-            final List<String> fileNamesToBeRecoveredFromSnapshot = snapshotFilesToRecover.getSnapshotFiles()
+            final List<String> fileNamesToBeRecoveredFromSnapshot = snapshotFilesToRecover.snapshotFiles()
                 .stream()
                 .map(fileInfo -> fileInfo.metadata().name())
                 .toList();
@@ -1207,8 +1207,8 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
                     BlobStoreIndexShardSnapshot.FileInfo snapshotFile,
                     ActionListener<Void> listener
                 ) {
-                    assertThat(repository, is(equalTo(snapshotFilesToRecover.getRepository())));
-                    assertThat(indexId, is(equalTo(snapshotFilesToRecover.getIndexId())));
+                    assertThat(repository, is(equalTo(snapshotFilesToRecover.repository())));
+                    assertThat(indexId, is(equalTo(snapshotFilesToRecover.indexId())));
                     assertThat(containsSnapshotFile(snapshotFilesToRecover, snapshotFile), is(equalTo(true)));
                     String fileName = snapshotFile.metadata().name();
 
@@ -1529,7 +1529,7 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
             final ShardRecoveryPlan fallbackPlan = shardRecoveryPlan.getFallbackPlan();
             List<StoreFileMetadata> sourceFilesToRecover = fallbackPlan.getSourceFilesToRecover();
             List<StoreFileMetadata> snapshotFilesToRecover = shardRecoveryPlan.getSnapshotFilesToRecover()
-                .getSnapshotFiles()
+                .snapshotFiles()
                 .stream()
                 .map(BlobStoreIndexShardSnapshot.FileInfo::metadata)
                 .toList();
@@ -1564,7 +1564,7 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
                         assertThat(receiveFileInfoFromSourceCalls.incrementAndGet(), is(equalTo(1)));
                     } else {
                         filesToRecover = shardRecoveryPlan.getSnapshotFilesToRecover()
-                            .getSnapshotFiles()
+                            .snapshotFiles()
                             .stream()
                             .map(BlobStoreIndexShardSnapshot.FileInfo::metadata)
                             .toList();
@@ -1697,7 +1697,7 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
         ShardRecoveryPlan.SnapshotFilesToRecover snapshotFilesToRecover,
         BlobStoreIndexShardSnapshot.FileInfo snapshotFile
     ) {
-        return snapshotFilesToRecover.getSnapshotFiles().stream().anyMatch(f -> f.metadata().isSame(snapshotFile.metadata()));
+        return snapshotFilesToRecover.snapshotFiles().stream().anyMatch(f -> f.metadata().isSame(snapshotFile.metadata()));
     }
 
     private boolean containsFile(List<StoreFileMetadata> filesMetadata, StoreFileMetadata storeFileMetadata) {
@@ -1758,7 +1758,7 @@ public class RecoverySourceHandlerTests extends MapperServiceTestCase {
         );
 
         Map<String, StoreFileMetadata> snapshotFiles = shardRecoveryPlan.getSnapshotFilesToRecover()
-            .getSnapshotFiles()
+            .snapshotFiles()
             .stream()
             .map(BlobStoreIndexShardSnapshot.FileInfo::metadata)
             .collect(Collectors.toMap(StoreFileMetadata::name, Function.identity()));

+ 1 - 1
server/src/test/java/org/elasticsearch/indices/recovery/RecoveryTests.java

@@ -439,7 +439,7 @@ public class RecoveryTests extends ESIndexLevelReplicationTestCase {
                     }
 
                     @Override
-                    public void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure) {
+                    public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) {
                         assertThat(ExceptionsHelper.unwrap(e, IOException.class).getMessage(), equalTo("simulated"));
                     }
                 });

+ 2 - 2
server/src/test/java/org/elasticsearch/recovery/RecoveriesCollectionTests.java

@@ -36,7 +36,7 @@ public class RecoveriesCollectionTests extends ESIndexLevelReplicationTestCase {
         }
 
         @Override
-        public void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure) {
+        public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) {
 
         }
     };
@@ -74,7 +74,7 @@ public class RecoveriesCollectionTests extends ESIndexLevelReplicationTestCase {
                     }
 
                     @Override
-                    public void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure) {
+                    public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) {
                         failed.set(true);
                         latch.countDown();
                     }

+ 1 - 1
test/framework/src/main/java/org/elasticsearch/index/shard/IndexShardTestCase.java

@@ -126,7 +126,7 @@ public abstract class IndexShardTestCase extends ESTestCase {
         }
 
         @Override
-        public void onRecoveryFailure(RecoveryState state, RecoveryFailedException e, boolean sendShardFailure) {
+        public void onRecoveryFailure(RecoveryFailedException e, boolean sendShardFailure) {
             throw new AssertionError(e);
         }
     };

+ 1 - 1
x-pack/plugin/snapshot-based-recoveries/src/main/java/org/elasticsearch/xpack/snapshotbasedrecoveries/recovery/plan/SnapshotsRecoveryPlannerService.java

@@ -162,7 +162,7 @@ public class SnapshotsRecoveryPlannerService implements RecoveryPlannerService {
             () -> Strings.format(
                 "%s attempting snapshot-based recovery of %s based on %s",
                 shardId,
-                snapshotFilesToRecover.getSnapshotFiles()
+                snapshotFilesToRecover.snapshotFiles()
                     .stream()
                     .map(BlobStoreIndexShardSnapshot.FileInfo::physicalName)
                     .collect(Collectors.joining(", ", "[", "]")),

+ 2 - 2
x-pack/plugin/snapshot-based-recoveries/src/test/java/org/elasticsearch/xpack/snapshotbasedrecoveries/recovery/plan/SnapshotsRecoveryPlannerServiceTests.java

@@ -526,8 +526,8 @@ public class SnapshotsRecoveryPlannerServiceTests extends ESTestCase {
     }
 
     private void assertUsesExpectedSnapshot(ShardRecoveryPlan shardRecoveryPlan, ShardSnapshot expectedSnapshotToUse) {
-        assertThat(shardRecoveryPlan.getSnapshotFilesToRecover().getIndexId(), equalTo(expectedSnapshotToUse.getIndexId()));
-        assertThat(shardRecoveryPlan.getSnapshotFilesToRecover().getRepository(), equalTo(expectedSnapshotToUse.getRepository()));
+        assertThat(shardRecoveryPlan.getSnapshotFilesToRecover().indexId(), equalTo(expectedSnapshotToUse.getIndexId()));
+        assertThat(shardRecoveryPlan.getSnapshotFilesToRecover().repository(), equalTo(expectedSnapshotToUse.getRepository()));
 
         final Store.MetadataSnapshot shardSnapshotMetadataSnapshot = expectedSnapshotToUse.getMetadataSnapshot();
         for (BlobStoreIndexShardSnapshot.FileInfo fileInfo : shardRecoveryPlan.getSnapshotFilesToRecover()) {