Skip to content

Commit 7fdc3b0

Browse files
Remove Incorrect Assertions from BlobStoreRepository (#69553) (#69593)
Follow-up to #69110. We can't assume that repository implementations will only throw `IOException`. The GCS SDK for example does throw `StorageException` which is not an IO exception so we must handle all exceptions the same way here.
1 parent f9b9df6 commit 7fdc3b0

File tree

1 file changed

+12
-26
lines changed

1 file changed

+12
-26
lines changed

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

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,43 +1043,29 @@ private List<String> cleanupStaleRootFiles(long previousGeneration, Collection<S
10431043
}
10441044
deleteFromContainer(blobContainer(), blobsToDelete);
10451045
return blobsToDelete;
1046-
} catch (IOException e) {
1046+
} catch (Exception e) {
10471047
logger.warn(() -> new ParameterizedMessage(
10481048
"[{}] The following blobs are no longer part of any snapshot [{}] but failed to remove them",
10491049
metadata.name(), blobsToDelete), e);
1050-
} catch (Exception e) {
1051-
// TODO: We shouldn't be blanket catching and suppressing all exceptions here and instead handle them safely upstream.
1052-
// Currently this catch exists as a stop gap solution to tackle unexpected runtime exceptions from implementations
1053-
// bubbling up and breaking the snapshot functionality.
1054-
assert false : e;
1055-
logger.warn(new ParameterizedMessage("[{}] Exception during cleanup of root level blobs", metadata.name()), e);
10561050
}
10571051
return Collections.emptyList();
10581052
}
10591053

10601054
private DeleteResult cleanupStaleIndices(Map<String, BlobContainer> foundIndices, Set<String> survivingIndexIds) {
10611055
DeleteResult deleteResult = DeleteResult.ZERO;
1062-
try {
1063-
for (Map.Entry<String, BlobContainer> indexEntry : foundIndices.entrySet()) {
1064-
final String indexSnId = indexEntry.getKey();
1065-
try {
1066-
if (survivingIndexIds.contains(indexSnId) == false) {
1067-
logger.debug("[{}] Found stale index [{}]. Cleaning it up", metadata.name(), indexSnId);
1068-
deleteResult = deleteResult.add(indexEntry.getValue().delete());
1069-
logger.debug("[{}] Cleaned up stale index [{}]", metadata.name(), indexSnId);
1070-
}
1071-
} catch (IOException e) {
1072-
logger.warn(() -> new ParameterizedMessage(
1073-
"[{}] index {} is no longer part of any snapshots in the repository, " +
1074-
"but failed to clean up their index folders", metadata.name(), indexSnId), e);
1056+
for (Map.Entry<String, BlobContainer> indexEntry : foundIndices.entrySet()) {
1057+
final String indexSnId = indexEntry.getKey();
1058+
try {
1059+
if (survivingIndexIds.contains(indexSnId) == false) {
1060+
logger.debug("[{}] Found stale index [{}]. Cleaning it up", metadata.name(), indexSnId);
1061+
deleteResult = deleteResult.add(indexEntry.getValue().delete());
1062+
logger.debug("[{}] Cleaned up stale index [{}]", metadata.name(), indexSnId);
10751063
}
1064+
} catch (Exception e) {
1065+
logger.warn(() -> new ParameterizedMessage(
1066+
"[{}] index {} is no longer part of any snapshot in the repository, " +
1067+
"but failed to clean up its index folder", metadata.name(), indexSnId), e);
10761068
}
1077-
} catch (Exception e) {
1078-
// TODO: We shouldn't be blanket catching and suppressing all exceptions here and instead handle them safely upstream.
1079-
// Currently this catch exists as a stop gap solution to tackle unexpected runtime exceptions from implementations
1080-
// bubbling up and breaking the snapshot functionality.
1081-
assert false : e;
1082-
logger.warn(new ParameterizedMessage("[{}] Exception during cleanup of stale indices", metadata.name()), e);
10831069
}
10841070
return deleteResult;
10851071
}

0 commit comments

Comments
 (0)