Skip to content

Commit

Permalink
Fix Edge-Case Threading Bug in TransportMountSearchableSnapshotAction (
Browse files Browse the repository at this point in the history
…#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 #68023
  • Loading branch information
original-brownbear authored May 18, 2021
1 parent 06fc62f commit 814839e
Showing 1 changed file with 6 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -185,9 +184,9 @@ protected void masterOperation(
final Repository repository = repositoriesService.repository(repoName);
SearchableSnapshots.getSearchableRepository(repository); // just check it's valid

final StepListener<RepositoryData> repositoryDataListener = new StepListener<>();
final ListenableFuture<RepositoryData> repositoryDataListener = new ListenableFuture<>();
repository.getRepositoryData(repositoryDataListener);
repositoryDataListener.whenComplete(repoData -> {
repositoryDataListener.addListener(ActionListener.wrap(repoData -> {
final Map<String, IndexId> indexIds = repoData.getIndices();
if (indexIds.containsKey(indexName) == false) {
throw new IndexNotFoundException("index [" + indexName + "] not found in repository [" + repoName + "]");
Expand Down Expand Up @@ -280,6 +279,6 @@ protected void masterOperation(
.snapshotUuid(snapshotId.getUUID()),
listener
);
}, listener::onFailure);
}, listener::onFailure), threadPool.executor(ThreadPool.Names.SNAPSHOT_META), null);
}
}

0 comments on commit 814839e

Please sign in to comment.