From 4849c3e1fe15cbb343b007be09319de2eeef9430 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 9 Jul 2019 13:06:09 +0200 Subject: [PATCH] Reduce Number of List Calls During Snapshot Create and Delete (#44088) * Reduce Numebr of List Calls During Snapshot Create and Delete Some obvious cleanups I found when investigation the API call count metering: * No need to get the latest generation id after loading latest repository data * Loading RepositoryData already requires fetching the latest generation so we can reuse it * Also, reuse list of all root blobs when fetching latest repo generation during snapshot delete like we do for shard folders * Lastly, don't try and load `index--1` (N = -1) repository data, it doesn't exist -> just return the empty repo data initially --- .../repositories/RepositoryData.java | 13 ++++ .../blobstore/BlobStoreRepository.java | 72 ++++++++++--------- .../blobstore/BlobStoreRepositoryTests.java | 6 +- 3 files changed, 54 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java index 3d0daff98936e..28606d6073b19 100644 --- a/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java +++ b/server/src/main/java/org/elasticsearch/repositories/RepositoryData.java @@ -174,6 +174,19 @@ public RepositoryData addSnapshot(final SnapshotId snapshotId, return new RepositoryData(genId, snapshots, newSnapshotStates, allIndexSnapshots, incompatibleSnapshotIds); } + /** + * Create a new instance with the given generation and all other fields equal to this instance. + * + * @param newGeneration New Generation + * @return New instance + */ + public RepositoryData withGenId(long newGeneration) { + if (newGeneration == genId) { + return this; + } + return new RepositoryData(newGeneration, this.snapshotIds, this.snapshotStates, this.indexSnapshots, incompatibleSnapshotIds); + } + /** * Remove a snapshot and remove any indices that no longer exist in the repository due to the deletion of the snapshot. */ 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 cac292ff66a69..63127dd673dcc 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -399,12 +399,12 @@ public void deleteSnapshot(SnapshotId snapshotId, long repositoryStateId, Action final Map foundIndices; final Set rootBlobs; try { - final RepositoryData repositoryData = getRepositoryData(); + rootBlobs = blobContainer().listBlobs().keySet(); + final RepositoryData repositoryData = getRepositoryData(latestGeneration(rootBlobs)); updatedRepositoryData = repositoryData.removeSnapshot(snapshotId); // Cache the indices that were found before writing out the new index-N blob so that a stuck master will never // delete an index that was created by another master node after writing this index-N blob. foundIndices = blobStore().blobContainer(basePath().add("indices")).children(); - rootBlobs = blobContainer().listBlobs().keySet(); writeIndexGen(updatedRepositoryData, repositoryStateId); } catch (Exception ex) { listener.onFailure(new RepositoryException(metadata.name(), "failed to delete snapshot [" + snapshotId + "]", ex)); @@ -691,7 +691,20 @@ public void endVerification(String seed) { @Override public RepositoryData getRepositoryData() { try { - final long indexGen = latestIndexBlobId(); + return getRepositoryData(latestIndexBlobId()); + } 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 determine repository generation from root blobs", ioe); + } + } + + private RepositoryData getRepositoryData(long indexGen) { + if (indexGen == RepositoryData.EMPTY_REPO_GEN) { + return RepositoryData.EMPTY; + } + try { final String snapshotsIndexBlobName = INDEX_FILE_PREFIX + Long.toString(indexGen); RepositoryData repositoryData; @@ -738,14 +751,14 @@ public boolean isReadOnly() { return readOnly; } - protected void writeIndexGen(final RepositoryData repositoryData, final long repositoryStateId) throws IOException { + protected void writeIndexGen(final RepositoryData repositoryData, final long expectedGen) throws IOException { assert isReadOnly() == false; // can not write to a read only repository - final long currentGen = latestIndexBlobId(); - if (currentGen != repositoryStateId) { + final long currentGen = repositoryData.getGenId(); + if (currentGen != expectedGen) { // the index file was updated by a concurrent operation, so we were operating on stale // repository data throw new RepositoryException(metadata.name(), "concurrent modification of the index-N file, expected current generation [" + - repositoryStateId + "], actual current generation [" + currentGen + + expectedGen + "], actual current generation [" + currentGen + "] - possibly due to simultaneous snapshot deletion requests"); } final long newGen = currentGen + 1; @@ -823,14 +836,15 @@ long readSnapshotIndexLatestBlob() throws IOException { } private long listBlobsToGetLatestIndexId() throws IOException { - Map blobs = blobContainer().listBlobsByPrefix(INDEX_FILE_PREFIX); + return latestGeneration(blobContainer().listBlobsByPrefix(INDEX_FILE_PREFIX).keySet()); + } + + private long latestGeneration(Collection rootBlobs) { long latest = RepositoryData.EMPTY_REPO_GEN; - if (blobs.isEmpty()) { - // no snapshot index blobs have been written yet - return latest; - } - for (final BlobMetaData blobMetaData : blobs.values()) { - final String blobName = blobMetaData.name(); + for (String blobName : rootBlobs) { + if (blobName.startsWith(INDEX_FILE_PREFIX) == false) { + continue; + } try { final long curr = Long.parseLong(blobName.substring(INDEX_FILE_PREFIX.length())); latest = Math.max(latest, curr); @@ -865,9 +879,9 @@ public void snapshotShard(Store store, MapperService mapperService, SnapshotId s throw new IndexShardSnapshotFailedException(shardId, "failed to list blobs", e); } - Tuple tuple = buildBlobStoreIndexShardSnapshots(blobs, shardContainer); + Tuple tuple = buildBlobStoreIndexShardSnapshots(blobs, shardContainer); BlobStoreIndexShardSnapshots snapshots = tuple.v1(); - int fileListGeneration = tuple.v2(); + long fileListGeneration = tuple.v2(); if (snapshots.snapshots().stream().anyMatch(sf -> sf.snapshot().equals(snapshotId.getName()))) { throw new IndexShardSnapshotFailedException(shardId, @@ -1069,9 +1083,9 @@ private void deleteShardSnapshot(IndexId indexId, ShardId snapshotShardId, Snaps throw new IndexShardSnapshotException(snapshotShardId, "Failed to list content of shard directory", e); } - Tuple tuple = buildBlobStoreIndexShardSnapshots(blobs, shardContainer); + Tuple tuple = buildBlobStoreIndexShardSnapshots(blobs, shardContainer); BlobStoreIndexShardSnapshots snapshots = tuple.v1(); - int fileListGeneration = tuple.v2(); + long fileListGeneration = tuple.v2(); try { indexShardSnapshotFormat.delete(shardContainer, snapshotId.getUUID()); @@ -1114,9 +1128,9 @@ private BlobStoreIndexShardSnapshot loadShardSnapshot(BlobContainer shardContain * @param blobs list of blobs in the container * @param reason a reason explaining why the shard index file is written */ - private void finalizeShard(List snapshots, int fileListGeneration, Map blobs, + private void finalizeShard(List snapshots, long fileListGeneration, Map blobs, String reason, BlobContainer shardContainer, ShardId shardId, SnapshotId snapshotId) { - final String indexGeneration = Integer.toString(fileListGeneration + 1); + final String indexGeneration = Long.toString(fileListGeneration + 1); try { final List blobsToDelete; if (snapshots.isEmpty()) { @@ -1150,26 +1164,14 @@ private void finalizeShard(List snapshots, int fileListGeneration * @param blobs list of blobs in repository * @return tuple of BlobStoreIndexShardSnapshots and the last snapshot index generation */ - private Tuple buildBlobStoreIndexShardSnapshots(Map blobs, + private Tuple buildBlobStoreIndexShardSnapshots(Map blobs, BlobContainer shardContainer) { - int latest = -1; Set blobKeys = blobs.keySet(); - for (String name : blobKeys) { - if (name.startsWith(SNAPSHOT_INDEX_PREFIX)) { - try { - int gen = Integer.parseInt(name.substring(SNAPSHOT_INDEX_PREFIX.length())); - if (gen > latest) { - latest = gen; - } - } catch (NumberFormatException ex) { - logger.warn("failed to parse index file name [{}]", name); - } - } - } + long latest = latestGeneration(blobKeys); if (latest >= 0) { try { final BlobStoreIndexShardSnapshots shardSnapshots = - indexShardSnapshotsFormat.read(shardContainer, Integer.toString(latest)); + indexShardSnapshotsFormat.read(shardContainer, Long.toString(latest)); return new Tuple<>(shardSnapshots, latest); } catch (IOException e) { final String file = SNAPSHOT_INDEX_PREFIX + latest; diff --git a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java index e2cb2fd071a11..f291609a0f8cb 100644 --- a/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java +++ b/server/src/test/java/org/elasticsearch/repositories/blobstore/BlobStoreRepositoryTests.java @@ -189,11 +189,13 @@ public void testRepositoryDataConcurrentModificationNotAllowed() throws IOExcept // write to index generational file RepositoryData repositoryData = generateRandomRepoData(); - repository.writeIndexGen(repositoryData, repositoryData.getGenId()); + final long startingGeneration = repositoryData.getGenId(); + repository.writeIndexGen(repositoryData, startingGeneration); // write repo data again to index generational file, errors because we already wrote to the // N+1 generation from which this repository data instance was created - expectThrows(RepositoryException.class, () -> repository.writeIndexGen(repositoryData, repositoryData.getGenId())); + expectThrows(RepositoryException.class, () -> repository.writeIndexGen( + repositoryData.withGenId(startingGeneration + 1), repositoryData.getGenId())); } public void testReadAndWriteIncompatibleSnapshots() throws Exception {