Skip to content

Commit

Permalink
Revert "Convert index path listeners to single path (#72511)" (#78935)
Browse files Browse the repository at this point in the history
This reverts commit fd138f5.

The revert was effectively conflict free, there was only a very minor
static import that needed fixing.

relates #78525
relates #71205
  • Loading branch information
rjernst authored Oct 11, 2021
1 parent 61ed2e7 commit 03a4577
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,12 +325,12 @@ public static class IndexFoldersDeletionListenerPlugin extends Plugin implements
public List<IndexFoldersDeletionListener> 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);
}
});
Expand Down
20 changes: 10 additions & 10 deletions server/src/main/java/org/elasticsearch/env/NodeEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ private static String toString(Collection<String> items) {
public void deleteShardDirectorySafe(
ShardId shardId,
IndexSettings indexSettings,
Consumer<Path> listener
Consumer<Path[]> listener
) throws IOException, ShardLockObtainFailedException {
final Path path = availableShardPath(shardId);
logger.trace("deleting shard {} directory, path: [{}]", shardId, path);
Expand Down Expand Up @@ -606,21 +606,21 @@ public static void acquireFSLockForPaths(IndexSettings indexSettings, Path... sh
public void deleteShardDirectoryUnderLock(
ShardLock lock,
IndexSettings indexSettings,
Consumer<Path> listener
Consumer<Path[]> 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);
Expand Down Expand Up @@ -676,7 +676,7 @@ public void deleteIndexDirectorySafe(
Index index,
long lockTimeoutMS,
IndexSettings indexSettings,
Consumer<Path> listener
Consumer<Path[]> listener
) throws IOException, ShardLockObtainFailedException {
final List<ShardLock> locks = lockAllForIndex(index, indexSettings, "deleting index directory", lockTimeoutMS);
try {
Expand All @@ -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<Path> listener) throws IOException {
public void deleteIndexDirectoryUnderLock(Index index, IndexSettings indexSettings, Consumer<Path[]> 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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public static void deleteLeftoverShardDirectory(
final NodeEnvironment env,
final ShardLock lock,
final IndexSettings indexSettings,
final Consumer<Path> listener
final Consumer<Path[]> listener
) throws IOException {
final String indexUUID = indexSettings.getUUID();
final Path path = env.availableShardPath(lock.getShardId());
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ public CompositeIndexFoldersDeletionListener(List<IndexStorePlugin.IndexFoldersD
}

@Override
public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path indexPath) {
public void beforeIndexFoldersDeleted(Index index, IndexSettings indexSettings, Path[] indexPaths) {
for (IndexStorePlugin.IndexFoldersDeletionListener listener : listeners) {
try {
listener.beforeIndexFoldersDeleted(index, indexSettings, indexPath);
listener.beforeIndexFoldersDeleted(index, indexSettings, indexPaths);
} catch (Exception e) {
assert false : new AssertionError(e);
throw e;
Expand All @@ -42,10 +42,10 @@ 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) {
for (IndexStorePlugin.IndexFoldersDeletionListener listener : listeners) {
try {
listener.beforeShardFoldersDeleted(shardId, indexSettings, shardPath);
listener.beforeShardFoldersDeleted(shardId, indexSettings, shardPaths);
} catch (Exception e) {
assert false : new AssertionError(e);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,22 +83,24 @@ default Map<String, RecoveryStateFactory> 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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,19 +200,21 @@ 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);
assertTrue(Files.exists(path.resolve("0")));
assertTrue(Files.exists(path.resolve("1")));

{
SetOnce<Path> listener = new SetOnce<>();
SetOnce<Path[]> 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);
Expand Down Expand Up @@ -261,9 +263,9 @@ protected void doRun() throws Exception {
start.countDown();
blockLatch.await();

final SetOnce<Path> listener = new SetOnce<>();
final SetOnce<Path[]> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down

0 comments on commit 03a4577

Please sign in to comment.