From 5603d2dcc983ed110e924d38a98253c78f4b2c73 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 23 Oct 2019 11:32:58 +0100 Subject: [PATCH] Cleanup Concurrent RepositoryData Loading (#48329) The loading of `RepositoryData` is not an atomic operation. It uses a list + get combination of calls. This lead to accidentally returning an empty repository data for generations >=0 which can never not exist unless the repository is corrupted. In the test #48122 (and other SLM tests) there was a low chance of running into this concurrent modification scenario and the repository actually moving two index generations between listing out the index-N and loading the latest version of it. Since we only keep two index-N around at a time this lead to unexpectedly absent snapshots in status APIs. Fixing the behavior to be more resilient is non-trivial but in the works. For now I think we should simply throw in this scenario. This will also help prevent corruption in the unlikely event but possible of running into this issue in a snapshot create or delete operation on master failover on a repository like S3 which doesn't have the "no overwrites" protection on writing a new index-N. Fixes #48122 --- .../repositories/blobstore/BlobStoreRepository.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java index 2de41fc0ef371..91c32cfa4493e 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -650,6 +650,9 @@ public void endVerification(String seed) { public RepositoryData getRepositoryData() { try { final long indexGen = latestIndexBlobId(); + if (indexGen == RepositoryData.EMPTY_REPO_GEN) { + return RepositoryData.EMPTY; + } final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); RepositoryData repositoryData; @@ -688,9 +691,6 @@ public RepositoryData getRepositoryData() { } } return repositoryData; - } catch (NoSuchFileException ex) { - // repository doesn't have an index blob, its a new blank repo - return RepositoryData.EMPTY; } catch (IOException ioe) { throw new RepositoryException(metadata.name(), "could not read repository data from index blob", ioe); }