From c824b855322e10bb01747189f1e59d224b18482e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Fri, 29 Mar 2019 20:23:43 +0100 Subject: [PATCH 1/4] Name Snapshot Data Blobs by UUID * There is no functional reason why we need incremental naming for these files but * As explained in #38941 it is a possible source of corrupting the repository * It wastes API calls for the list operation * Is just needless complication * Since we store the exact names of the data blobs in all the metadata anyway, we can make this change without any BwC considerations * Even on the worst case scenario of a downgrade the functionality would continue working since the incremental names wouldn't conflict with the uuids and the number parsing for finding the next incremental name suppresses the exception when encountring a non-numeric value after the double underscore prefix --- .../blobstore/BlobStoreRepository.java | 38 +------------------ 1 file changed, 1 insertion(+), 37 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 a73626351d875..cf5a195dba888 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -1069,41 +1069,6 @@ protected void finalize(final List snapshots, } } - /** - * Generates blob name - * - * @param generation the blob number - * @return the blob name - */ - protected String fileNameFromGeneration(long generation) { - return DATA_BLOB_PREFIX + Long.toString(generation, Character.MAX_RADIX); - } - - /** - * Finds the next available blob number - * - * @param blobs list of blobs in the repository - * @return next available blob number - */ - protected long findLatestFileNameGeneration(Map blobs) { - long generation = -1; - for (String name : blobs.keySet()) { - if (!name.startsWith(DATA_BLOB_PREFIX)) { - continue; - } - name = canonicalName(name); - try { - long currentGen = Long.parseLong(name.substring(DATA_BLOB_PREFIX.length()), Character.MAX_RADIX); - if (currentGen > generation) { - generation = currentGen; - } - } catch (NumberFormatException e) { - logger.warn("file [{}] does not conform to the '{}' schema", name, DATA_BLOB_PREFIX); - } - } - return generation; - } - /** * Loads all available snapshots in the repository * @@ -1196,7 +1161,6 @@ public void snapshot(final IndexCommit snapshotIndexCommit) { throw new IndexShardSnapshotFailedException(shardId, "failed to list blobs", e); } - long generation = findLatestFileNameGeneration(blobs); Tuple tuple = buildBlobStoreIndexShardSnapshots(blobs); BlobStoreIndexShardSnapshots snapshots = tuple.v1(); int fileListGeneration = tuple.v2(); @@ -1264,7 +1228,7 @@ public void snapshot(final IndexCommit snapshotIndexCommit) { indexIncrementalSize += md.length(); // create a new FileInfo BlobStoreIndexShardSnapshot.FileInfo snapshotFileInfo = - new BlobStoreIndexShardSnapshot.FileInfo(fileNameFromGeneration(++generation), md, chunkSize()); + new BlobStoreIndexShardSnapshot.FileInfo(DATA_BLOB_PREFIX + UUIDs.randomBase64UUID(), md, chunkSize()); indexCommitPointFiles.add(snapshotFileInfo); filesToSnapshot.add(snapshotFileInfo); } else { From c09e64837cd0df921b90af70b18b9efbc680b620 Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 1 Apr 2019 16:01:48 +0200 Subject: [PATCH 2/4] CR: update Javadoc --- .../repositories/blobstore/BlobStoreRepository.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 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 cf5a195dba888..0372beede4b68 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -135,11 +135,11 @@ * |- Ac1342-B_x/ - data for index "foo" which was assigned the unique id of Ac1342-B_x in the repository * | |- meta-20131010.dat - JSON Serialized IndexMetaData for index "foo" * | |- 0/ - data for shard "0" of index "foo" - * | | |- __1 \ - * | | |- __2 | - * | | |- __3 |- files from different segments see snapshot-* for their mappings to real segment files - * | | |- __4 | - * | | |- __5 / + * | | |- __1 \ + * | | |- __2 | + * | | |- __VPO5oDMVT5y4Akv8T_AO_A |- files from different segments see snapshot-* for their mappings to real segment files + * | | |- __1gbJy18wS_2kv1qI7FgKuQ | (files with numeric names were created by older ES versions) + * | | |- __R8JvZAHlSMyMXyZc2SS8Zg / * | | ..... * | | |- snap-20131010.dat - JSON serialized BlobStoreIndexShardSnapshot for snapshot "20131010" * | | |- snap-20131011.dat - JSON serialized BlobStoreIndexShardSnapshot for snapshot "20131011" From 343efe6bd9eb512ee6f7e815243fb3071678ee16 Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 1 Apr 2019 17:04:31 +0200 Subject: [PATCH 3/4] fix prefix --- .../repositories/blobstore/BlobStoreRepository.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 0372beede4b68..915ef91fcc9dd 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -137,7 +137,7 @@ * | |- 0/ - data for shard "0" of index "foo" * | | |- __1 \ * | | |- __2 | - * | | |- __VPO5oDMVT5y4Akv8T_AO_A |- files from different segments see snapshot-* for their mappings to real segment files + * | | |- __VPO5oDMVT5y4Akv8T_AO_A |- files from different segments see snap-* for their mappings to real segment files * | | |- __1gbJy18wS_2kv1qI7FgKuQ | (files with numeric names were created by older ES versions) * | | |- __R8JvZAHlSMyMXyZc2SS8Zg / * | | ..... From 7c844b410557d1cafbf6ad89def0a924576ef145 Mon Sep 17 00:00:00 2001 From: Armin Date: Tue, 2 Apr 2019 10:43:35 +0200 Subject: [PATCH 4/4] CR: move comment about legacy naming --- .../repositories/blobstore/BlobStoreRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 63e8cf86335f2..253501a542b20 100644 --- a/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java +++ b/server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java @@ -135,10 +135,10 @@ * |- Ac1342-B_x/ - data for index "foo" which was assigned the unique id of Ac1342-B_x in the repository * | |- meta-20131010.dat - JSON Serialized IndexMetaData for index "foo" * | |- 0/ - data for shard "0" of index "foo" - * | | |- __1 \ + * | | |- __1 \ (files with numeric names were created by older ES versions) * | | |- __2 | * | | |- __VPO5oDMVT5y4Akv8T_AO_A |- files from different segments see snap-* for their mappings to real segment files - * | | |- __1gbJy18wS_2kv1qI7FgKuQ | (files with numeric names were created by older ES versions) + * | | |- __1gbJy18wS_2kv1qI7FgKuQ | * | | |- __R8JvZAHlSMyMXyZc2SS8Zg / * | | ..... * | | |- snap-20131010.dat - JSON serialized BlobStoreIndexShardSnapshot for snapshot "20131010"