Skip to content

Commit

Permalink
Fix unhandled exception when blobstore repository contains unexpected…
Browse files Browse the repository at this point in the history
… file (#93914) (#97113)

If there's any file with the `index-` prefix but not a number after that at the repo root we
must not throw here. If we do, we will end up throwing an unexpected exception that is not
properly handled by `org.elasticsearch.snapshots.SnapshotsService#failAllListenersOnMasterFailOver`,
leading to the repository generation not getting correctly set in the cluster state down the line.
  • Loading branch information
joegallo committed Jun 27, 2023
1 parent 6cd68fc commit 62579b0
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
5 changes: 5 additions & 0 deletions docs/changelog/93914.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 93914
summary: Fix unhandled exception when blobstore repository contains unexpected file
area: Snapshot/Restore
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.repositories.ShardGeneration;
import org.elasticsearch.repositories.ShardGenerations;
import org.elasticsearch.repositories.blobstore.BlobStoreRepository;
import org.elasticsearch.repositories.fs.FsRepository;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xcontent.XContentFactory;

Expand Down Expand Up @@ -844,6 +845,19 @@ public void testSnapshotWithMissingShardLevelIndexFile() throws Exception {
assertEquals(0, restoreSnapshotResponse.getRestoreInfo().failedShards());
}

public void testDeletesWithUnexpectedIndexBlob() throws Exception {
Path repo = randomRepoPath();
final String repoName = "test-repo";
createRepository(repoName, FsRepository.TYPE, repo);
final String snapshot = "first-snapshot";
createFullSnapshot(repoName, snapshot);
// create extra file with the index prefix
final Path extraFile = Files.createFile(repo.resolve(BlobStoreRepository.INDEX_FILE_PREFIX + randomAlphaOfLength(5)));
assertAcked(startDeleteSnapshot(repoName, snapshot).get());
// delete file again so repo consistency checks pass
Files.delete(extraFile);
}

private void assertRepositoryBlocked(Client client, String repo, String existingSnapshot) {
logger.info("--> try to delete snapshot");
final RepositoryException repositoryException3 = expectThrows(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,12 @@ private static List<String> staleRootBlobs(RepositoryData repositoryData, Set<St
return allSnapshotIds.contains(foundUUID) == false;
} else if (blob.startsWith(INDEX_FILE_PREFIX)) {
// TODO: Include the current generation here once we remove keeping index-(N-1) around from #writeIndexGen
return repositoryData.getGenId() > Long.parseLong(blob.substring(INDEX_FILE_PREFIX.length()));
try {
return repositoryData.getGenId() > Long.parseLong(blob.substring(INDEX_FILE_PREFIX.length()));
} catch (NumberFormatException nfe) {
// odd case of an extra file with the index- prefix that we can't identify
return false;
}
}
return false;
}).collect(Collectors.toList());
Expand Down

0 comments on commit 62579b0

Please sign in to comment.