From 38168a6a332b7755f9d03ce55b011b7ab0ec1ea9 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 18 May 2021 13:55:42 +0200 Subject: [PATCH 1/3] Fix Edge-Case Threading Bug in TransportMountSearchableSnapshotAction 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 --- .../action/TransportMountSearchableSnapshotAction.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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..31282d35a71e3 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; @@ -185,9 +185,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 +280,6 @@ protected void masterOperation( .snapshotUuid(snapshotId.getUUID()), listener ); - }, listener::onFailure); + }, listener::onFailure), threadPool.generic(), null); } } From cd29039fff086aeeef8d926ff1795bc3181d5a15 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 18 May 2021 14:48:13 +0200 Subject: [PATCH 2/3] use meta pool --- .../action/TransportMountSearchableSnapshotAction.java | 7 +++---- 1 file changed, 3 insertions(+), 4 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 31282d35a71e3..693a1c82f6ecc 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 @@ -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 mainly loading repository metadata in this action + ThreadPool.Names.SNAPSHOT_META ); this.client = client; this.repositoriesService = repositoriesService; @@ -280,6 +279,6 @@ protected void masterOperation( .snapshotUuid(snapshotId.getUUID()), listener ); - }, listener::onFailure), threadPool.generic(), null); + }, listener::onFailure), threadPool.executor(ThreadPool.Names.SNAPSHOT_META), null); } } From 824f869a4628792a95462e0b9741e9e6b8770f87 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Tue, 18 May 2021 14:49:04 +0200 Subject: [PATCH 3/3] use meta pool --- .../action/TransportMountSearchableSnapshotAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 693a1c82f6ecc..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 @@ -103,7 +103,7 @@ public TransportMountSearchableSnapshotAction( MountSearchableSnapshotRequest::new, indexNameExpressionResolver, RestoreSnapshotResponse::new, - // Use SNAPSHOT_META pool since we are mainly loading repository metadata in this action + // Use SNAPSHOT_META pool since we are slow due to loading repository metadata in this action ThreadPool.Names.SNAPSHOT_META ); this.client = client;