From 814839e70431edf88999e27a1d830579c6bd2d3a Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 18 May 2021 16:19:00 +0200 Subject: [PATCH] Fix Edge-Case Threading Bug in TransportMountSearchableSnapshotAction (#73196) The callback to loading the repository-data may not run on generic in the uncached case because of the repo data deduplication logic. The same issue was fixed for the snapshot status API in https://github.com/elastic/elasticsearch/pull/68023 --- .../TransportMountSearchableSnapshotAction.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java index eba6db3789bbe..b4ab73c72be8c 100644 --- a/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java +++ b/x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/action/TransportMountSearchableSnapshotAction.java @@ -10,7 +10,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.action.StepListener; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotRequest; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; import org.elasticsearch.action.support.ActionFilters; @@ -26,6 +25,7 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ListenableFuture; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.ShardLimitValidator; import org.elasticsearch.indices.SystemIndices; @@ -103,9 +103,8 @@ public TransportMountSearchableSnapshotAction( MountSearchableSnapshotRequest::new, indexNameExpressionResolver, RestoreSnapshotResponse::new, - // Avoid SNAPSHOT since snapshot threads may all be busy with long-running tasks which would block this action from responding - // with an error. Avoid SAME since getting the repository metadata may block on IO. - ThreadPool.Names.GENERIC + // Use SNAPSHOT_META pool since we are slow due to loading repository metadata in this action + ThreadPool.Names.SNAPSHOT_META ); this.client = client; this.repositoriesService = repositoriesService; @@ -185,9 +184,9 @@ protected void masterOperation( final Repository repository = repositoriesService.repository(repoName); SearchableSnapshots.getSearchableRepository(repository); // just check it's valid - final StepListener repositoryDataListener = new StepListener<>(); + final ListenableFuture repositoryDataListener = new ListenableFuture<>(); repository.getRepositoryData(repositoryDataListener); - repositoryDataListener.whenComplete(repoData -> { + repositoryDataListener.addListener(ActionListener.wrap(repoData -> { final Map indexIds = repoData.getIndices(); if (indexIds.containsKey(indexName) == false) { throw new IndexNotFoundException("index [" + indexName + "] not found in repository [" + repoName + "]"); @@ -280,6 +279,6 @@ protected void masterOperation( .snapshotUuid(snapshotId.getUUID()), listener ); - }, listener::onFailure); + }, listener::onFailure), threadPool.executor(ThreadPool.Names.SNAPSHOT_META), null); } }