Skip to content

Commit f115524

Browse files
Fix Needless Warnings when Restoring over Closed Index (#75912) (#75926)
We're trying to delete each file twice and will always warn on the second non-suppressing delete method when there's files to delete.
1 parent 6ee2546 commit f115524

File tree

3 files changed

+38
-3
lines changed

3 files changed

+38
-3
lines changed

server/src/internalClusterTest/java/org/elasticsearch/snapshots/RestoreSnapshotIT.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
package org.elasticsearch.snapshots;
1010

11+
import org.apache.logging.log4j.Level;
12+
import org.apache.logging.log4j.LogManager;
13+
import org.apache.logging.log4j.Logger;
1114
import org.elasticsearch.Version;
1215
import org.elasticsearch.action.ActionFuture;
1316
import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse;
@@ -18,14 +21,17 @@
1821
import org.elasticsearch.cluster.block.ClusterBlocks;
1922
import org.elasticsearch.cluster.metadata.IndexMetadata;
2023
import org.elasticsearch.cluster.metadata.MappingMetadata;
24+
import org.elasticsearch.common.logging.Loggers;
2125
import org.elasticsearch.common.settings.Settings;
2226
import org.elasticsearch.common.unit.ByteSizeUnit;
2327
import org.elasticsearch.common.xcontent.XContentFactory;
2428
import org.elasticsearch.core.TimeValue;
2529
import org.elasticsearch.indices.InvalidIndexNameException;
2630
import org.elasticsearch.repositories.RepositoriesService;
31+
import org.elasticsearch.repositories.blobstore.FileRestoreContext;
2732
import org.elasticsearch.repositories.fs.FsRepository;
2833
import org.elasticsearch.rest.RestStatus;
34+
import org.elasticsearch.test.MockLogAppender;
2935

3036
import java.nio.file.Path;
3137
import java.util.Arrays;
@@ -878,4 +884,34 @@ public void testFailOnAncientVersion() throws Exception {
878884
)
879885
);
880886
}
887+
888+
public void testNoWarningsOnRestoreOverClosedIndex() throws IllegalAccessException {
889+
final String repoName = "test-repo";
890+
createRepository(repoName, FsRepository.TYPE);
891+
final String indexName = "test-idx";
892+
createIndexWithContent(indexName);
893+
final String snapshotName = "test-snapshot";
894+
createSnapshot(repoName, snapshotName, org.elasticsearch.core.List.of(indexName));
895+
index(indexName, "_doc", "some_id", org.elasticsearch.core.Map.of("foo", "bar"));
896+
assertAcked(admin().indices().prepareClose(indexName).get());
897+
final MockLogAppender mockAppender = new MockLogAppender();
898+
mockAppender.addExpectation(
899+
new MockLogAppender.UnseenEventExpectation("no warnings", FileRestoreContext.class.getCanonicalName(), Level.WARN, "*")
900+
);
901+
mockAppender.start();
902+
final Logger logger = LogManager.getLogger(FileRestoreContext.class);
903+
Loggers.addAppender(logger, mockAppender);
904+
try {
905+
final RestoreSnapshotResponse restoreSnapshotResponse = clusterAdmin().prepareRestoreSnapshot(repoName, snapshotName)
906+
.setIndices(indexName)
907+
.setRestoreGlobalState(false)
908+
.setWaitForCompletion(true)
909+
.get();
910+
assertEquals(0, restoreSnapshotResponse.getRestoreInfo().failedShards());
911+
mockAppender.assertAllExpectationsMatched();
912+
} finally {
913+
Loggers.removeAppender(logger, mockAppender);
914+
mockAppender.stop();
915+
}
916+
}
881917
}

server/src/main/java/org/elasticsearch/index/store/Store.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,7 @@ public String toString() {
11321132

11331133
/**
11341134
* Returns true if the file is auto-generated by the store and shouldn't be deleted during cleanup.
1135-
* This includes write lock and checksum files
1135+
* This includes write lock files
11361136
*/
11371137
public static boolean isAutogenerated(String name) {
11381138
return IndexWriter.WRITE_LOCK_NAME.equals(name);

server/src/main/java/org/elasticsearch/repositories/blobstore/FileRestoreContext.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,10 +190,9 @@ private void afterRestore(SnapshotFiles snapshotFiles, Store store, StoreFileMet
190190
try {
191191
for (String storeFile : store.directory().listAll()) {
192192
if (Store.isAutogenerated(storeFile) || snapshotFiles.containPhysicalIndexFile(storeFile)) {
193-
continue; // skip write.lock, checksum files and files that exist in the snapshot
193+
continue; // skip write.lock and files that exist in the snapshot
194194
}
195195
try {
196-
store.deleteQuiet("restore", storeFile);
197196
store.directory().deleteFile(storeFile);
198197
} catch (ImmutableDirectoryException e) {
199198
// snapshots of immutable directories only contain an empty `segments_N` file since the data lives elsewhere, and if we

0 commit comments

Comments
 (0)