Skip to content

Commit

Permalink
Move Get Snapshots Serialization to Management Pool (#83215)
Browse files Browse the repository at this point in the history
It's in the title. For large index counts generating the response,
both for transport as well as REST layer gets quite expensive.
Better generate it off the transport threads.
  • Loading branch information
original-brownbear committed Jan 31, 2022
1 parent 997a600 commit 50fcaac
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 4 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/83215.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 83215
summary: Move Get Snapshots Serialization to Management Pool
area: Snapshot/Restore
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
registerHandler.accept(new RestDeleteRepositoryAction());
registerHandler.accept(new RestVerifyRepositoryAction());
registerHandler.accept(new RestCleanupRepositoryAction());
registerHandler.accept(new RestGetSnapshotsAction());
registerHandler.accept(new RestGetSnapshotsAction(threadPool));
registerHandler.accept(new RestCreateSnapshotAction());
registerHandler.accept(new RestCloneSnapshotAction());
registerHandler.accept(new RestRestoreSnapshotAction());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ public TransportGetSnapshotsAction(
GetSnapshotsRequest::new,
indexNameExpressionResolver,
GetSnapshotsResponse::new,
ThreadPool.Names.SAME
ThreadPool.Names.MANAGEMENT // Execute this on the management pool because creating the response can become fairly expensive
// for large repositories in the verbose=false case when there are a lot of indices per snapshot.
// This is intentionally not using the snapshot_meta pool because that pool is sized rather large
// to accommodate concurrent IO and could consume excessive CPU resources through concurrent
// verbose=false requests that are CPU bound only.
);
this.repositoriesService = repositoriesService;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.DispatchingRestToXContentListener;
import org.elasticsearch.rest.action.RestCancellableNodeClient;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.search.sort.SortOrder;
import org.elasticsearch.threadpool.ThreadPool;

import java.io.IOException;
import java.util.List;
Expand All @@ -31,6 +32,12 @@
*/
public class RestGetSnapshotsAction extends BaseRestHandler {

private final ThreadPool threadPool;

public RestGetSnapshotsAction(ThreadPool threadPool) {
this.threadPool = threadPool;
}

@Override
public List<Route> routes() {
return List.of(new Route(GET, "/_snapshot/{repository}/{snapshot}"));
Expand Down Expand Up @@ -76,6 +83,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getSnapshotsRequest.masterNodeTimeout()));
return channel -> new RestCancellableNodeClient(client, request.getHttpChannel()).admin()
.cluster()
.getSnapshots(getSnapshotsRequest, new RestToXContentListener<>(channel));
.getSnapshots(
getSnapshotsRequest,
new DispatchingRestToXContentListener<>(threadPool.executor(ThreadPool.Names.MANAGEMENT), channel, request)
);
}
}

0 comments on commit 50fcaac

Please sign in to comment.