From 03a45770ef73e8523947dbea8572c4cd189816ff Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 11 Oct 2021 13:44:45 -0700 Subject: [PATCH] Revert "Convert index path listeners to single path (#72511)" (#78935) This reverts commit fd138f578b9bfc434fa3736d58e0c8a8433573cd. The revert was effectively conflict free, there was only a very minor static import that needed fixing. relates #78525 relates #71205 --- .../IndexFoldersDeletionListenerIT.java | 4 ++-- .../elasticsearch/env/NodeEnvironment.java | 20 +++++++++---------- .../org/elasticsearch/index/IndexService.java | 4 ++-- .../elasticsearch/index/shard/ShardPath.java | 4 ++-- .../elasticsearch/indices/IndicesService.java | 2 +- ...CompositeIndexFoldersDeletionListener.java | 8 ++++---- .../plugins/IndexStorePlugin.java | 14 +++++++------ .../env/NodeEnvironmentTests.java | 16 ++++++++------- .../elasticsearch/index/IndexModuleTests.java | 4 ++-- ...eSnapshotIndexFoldersDeletionListener.java | 4 ++-- 10 files changed, 42 insertions(+), 38 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java index 646af47ca3dc7..7f44fe47a19b9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/plugins/IndexFoldersDeletionListenerIT.java @@ -325,12 +325,12 @@ public static class IndexFoldersDeletionListenerPlugin extends Plugin implements public List getIndexFoldersDeletionListeners() { return List.of(new IndexFoldersDeletionListener() { @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path indexPath) { + public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) { deletedIndices.add(index); } @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path shardPath) { + public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) { deletedShards.computeIfAbsent(shardId.getIndex(), i -> new ArrayList<>()).add(shardId); } }); diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index be5cf4578caf5..1771a79e778c9 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -553,7 +553,7 @@ private static String toString(Collection items) { public void deleteShardDirectorySafe( ShardId shardId, IndexSettings indexSettings, - Consumer listener + Consumer listener ) throws IOException, ShardLockObtainFailedException { final Path path = availableShardPath(shardId); logger.trace("deleting shard {} directory, path: [{}]", shardId, path); @@ -606,21 +606,21 @@ public static void acquireFSLockForPaths(IndexSettings indexSettings, Path... sh public void deleteShardDirectoryUnderLock( ShardLock lock, IndexSettings indexSettings, - Consumer listener + Consumer listener ) throws IOException { final ShardId shardId = lock.getShardId(); assert isShardLocked(shardId) : "shard " + shardId + " is not locked"; final Path path = availableShardPath(shardId); logger.trace("acquiring locks for {}, path: [{}]", shardId, path); acquireFSLockForPaths(indexSettings, path); - listener.accept(path); + listener.accept(new Path[] { path }); IOUtils.rm(path); if (indexSettings.hasCustomDataPath()) { Path customLocation = resolveCustomLocation(indexSettings.customDataPath(), shardId); logger.trace("acquiring lock for {}, custom path: [{}]", shardId, customLocation); acquireFSLockForPaths(indexSettings, customLocation); logger.trace("deleting custom shard {} directory [{}]", shardId, customLocation); - listener.accept(customLocation); + listener.accept(new Path[]{customLocation}); IOUtils.rm(customLocation); } logger.trace("deleted shard {} directory, path: [{}]", shardId, path); @@ -676,7 +676,7 @@ public void deleteIndexDirectorySafe( Index index, long lockTimeoutMS, IndexSettings indexSettings, - Consumer listener + Consumer listener ) throws IOException, ShardLockObtainFailedException { final List locks = lockAllForIndex(index, indexSettings, "deleting index directory", lockTimeoutMS); try { @@ -689,19 +689,19 @@ public void deleteIndexDirectorySafe( /** * Deletes an indexes data directory recursively. * Note: this method assumes that the shard lock is acquired - * @param index the index to delete + * + * @param index the index to delete * @param indexSettings settings for the index being deleted - * @param listener */ - public void deleteIndexDirectoryUnderLock(Index index, IndexSettings indexSettings, Consumer listener) throws IOException { + public void deleteIndexDirectoryUnderLock(Index index, IndexSettings indexSettings, Consumer listener) throws IOException { final Path indexPath = indexPath(index); logger.trace("deleting index {} directory: [{}]", index, indexPath); - listener.accept(indexPath); + listener.accept(new Path[] { indexPath }); IOUtils.rm(indexPath); if (indexSettings.hasCustomDataPath()) { Path customLocation = resolveIndexCustomLocation(indexSettings.customDataPath(), index.getUUID()); logger.trace("deleting custom index {} directory [{}]", index, customLocation); - listener.accept(customLocation); + listener.accept(new Path[]{customLocation}); IOUtils.rm(customLocation); } } diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 5585211939e32..0e1e68f5c3e75 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -411,8 +411,8 @@ public synchronized IndexShard createShard( } catch (IllegalStateException ex) { logger.warn("{} failed to load shard path, trying to remove leftover", shardId); try { - ShardPath.deleteLeftoverShardDirectory(logger, nodeEnv, lock, this.indexSettings, shardPath -> - indexFoldersDeletionListener.beforeShardFoldersDeleted(shardId, this.indexSettings, shardPath)); + ShardPath.deleteLeftoverShardDirectory(logger, nodeEnv, lock, this.indexSettings, shardPaths -> + indexFoldersDeletionListener.beforeShardFoldersDeleted(shardId, this.indexSettings, shardPaths)); path = ShardPath.loadShardPath(logger, nodeEnv, shardId, this.indexSettings.customDataPath()); } catch (Exception inner) { ex.addSuppressed(inner); diff --git a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java index 8cd633e143626..bfbe0d0f26c6d 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java +++ b/server/src/main/java/org/elasticsearch/index/shard/ShardPath.java @@ -167,7 +167,7 @@ public static void deleteLeftoverShardDirectory( final NodeEnvironment env, final ShardLock lock, final IndexSettings indexSettings, - final Consumer listener + final Consumer listener ) throws IOException { final String indexUUID = indexSettings.getUUID(); final Path path = env.availableShardPath(lock.getShardId()); @@ -178,7 +178,7 @@ public static void deleteLeftoverShardDirectory( logger.warn("{} deleting leftover shard on path: [{}] with a different index UUID", lock.getShardId(), path); assert Files.isDirectory(path) : path + " is not a directory"; NodeEnvironment.acquireFSLockForPaths(indexSettings, path); - listener.accept(path); + listener.accept(new Path[]{path}); IOUtils.rm(path); } } diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesService.java b/server/src/main/java/org/elasticsearch/indices/IndicesService.java index d60bce286f8f7..4dd60bade76e2 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -981,7 +981,7 @@ public void deleteShardStore(String reason, ShardId shardId, ClusterState cluste throw new IllegalStateException("Can't delete shard " + shardId + " (cause: " + shardDeletionCheckResult + ")"); } nodeEnv.deleteShardDirectorySafe(shardId, indexSettings, - path -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, path)); + paths -> indexFoldersDeletionListeners.beforeShardFoldersDeleted(shardId, indexSettings, paths)); logger.debug("{} deleted shard reason [{}]", shardId, reason); if (canDeleteIndexContents(shardId.getIndex())) { diff --git a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java index 5df5e9dd22ea9..16ea2601e48c8 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java +++ b/server/src/main/java/org/elasticsearch/indices/store/CompositeIndexFoldersDeletionListener.java @@ -30,10 +30,10 @@ public CompositeIndexFoldersDeletionListener(List getRecoveryStateFactories() { */ interface IndexFoldersDeletionListener { /** - * Invoked before the folders of an index are deleted from disk. The folder's {@link Path} may or may not + * Invoked before the folders of an index are deleted from disk. The list of folders contains {@link Path}s that may or may not * exist on disk. Shard locks are expected to be acquired at the time this method is invoked. + * * @param index the {@link Index} of the index whose folders are going to be deleted * @param indexSettings settings for the index whose folders are going to be deleted - * @param indexPath the path of the folders that will be deleted + * @param indexPaths the paths of the folders that are going to be deleted */ - void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path indexPath); + void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths); /** - * Invoked before the folders of a shard are deleted from disk. The folder's {@link Path} may or may not + * Invoked before the folders of a shard are deleted from disk. The list of folders contains {@link Path}s that may or may not * exist on disk. Shard locks are expected to be acquired at the time this method is invoked. + * * @param shardId the {@link ShardId} of the shard whose folders are going to be deleted * @param indexSettings index settings of the shard whose folders are going to be deleted - * @param shardPath the path of the folder that will be deleted + * @param shardPaths the paths of the folders that are going to be deleted */ - void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path shardPath); + void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths); } /** diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 3153aac062fa6..8afc357b431eb 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -200,8 +200,8 @@ public void testDeleteSafe() throws Exception { Files.createDirectories(path.resolve("1")); expectThrows(ShardLockObtainFailedException.class, - () -> env.deleteShardDirectorySafe(new ShardId(index, 0), idxSettings, shardPath -> { - assert false : "should not be called " + shardPath; + () -> env.deleteShardDirectorySafe(new ShardId(index, 0), idxSettings, shardPaths -> { + assert false : "should not be called " + shardPaths; })); path = env.indexPath(index); @@ -209,10 +209,12 @@ public void testDeleteSafe() throws Exception { assertTrue(Files.exists(path.resolve("1"))); { - SetOnce listener = new SetOnce<>(); + SetOnce listener = new SetOnce<>(); env.deleteShardDirectorySafe(new ShardId(index, 1), idxSettings, listener::set); - Path deletedPath = listener.get(); - assertThat(deletedPath, equalTo(env.nodePaths()[0].resolve(index).resolve("1"))); + Path[] deletedPaths = listener.get(); + for (int i = 0; i < env.nodePaths().length; i++) { + assertThat(deletedPaths[i], equalTo(env.nodePaths()[i].resolve(index).resolve("1"))); + } } path = env.indexPath(index); @@ -261,9 +263,9 @@ protected void doRun() throws Exception { start.countDown(); blockLatch.await(); - final SetOnce listener = new SetOnce<>(); + final SetOnce listener = new SetOnce<>(); env.deleteIndexDirectorySafe(index, 5000, idxSettings, listener::set); - assertThat(listener.get(), equalTo(env.indexPath(index))); + assertThat(listener.get()[0], equalTo(env.indexPath(index))); assertNull(threadException.get()); path = env.indexPath(index); diff --git a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java index 1296d8c9d8d62..dbb159953a4c6 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexModuleTests.java @@ -122,11 +122,11 @@ public void addPendingDelete(ShardId shardId, IndexSettings indexSettings) { private IndexStorePlugin.IndexFoldersDeletionListener indexDeletionListener = new IndexStorePlugin.IndexFoldersDeletionListener() { @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path indexPath) { + public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) { } @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path shardPath) { + public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) { } }; diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java index 5090c3e475804..bc41b2c7f0c4c 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/allocation/SearchableSnapshotIndexFoldersDeletionListener.java @@ -45,7 +45,7 @@ public SearchableSnapshotIndexFoldersDeletionListener( } @Override - public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path indexPath) { + public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) { if (isSearchableSnapshotStore(indexSettings.getSettings())) { for (int shard = 0; shard < indexSettings.getNumberOfShards(); shard++) { markShardAsEvictedInCache(new ShardId(index, shard), indexSettings); @@ -54,7 +54,7 @@ public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, } @Override - public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path shardPath) { + public void beforeShardFoldersDeleted(ShardId shardId, IndexSettings indexSettings, Path[] shardPaths) { if (isSearchableSnapshotStore(indexSettings.getSettings())) { markShardAsEvictedInCache(shardId, indexSettings); }