[Transport-gRPC] Wire gRPC call cancellation into search task cancellation#22429
[Transport-gRPC] Wire gRPC call cancellation into search task cancellation#22429pseudo-nymous wants to merge 1 commit into
Conversation
Signed-off-by: pseudo-nymous <amirraza8681@gmail.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #22429 +/- ##
============================================
+ Coverage 73.39% 73.47% +0.07%
- Complexity 76397 76503 +106
============================================
Files 6105 6105
Lines 346613 346616 +3
Branches 49888 49888
============================================
+ Hits 254403 254664 +261
+ Misses 71958 71689 -269
- Partials 20252 20263 +11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Description
Cancels the server-side search task when a gRPC search call is cancelled (client cancels the RPC, disconnects, or the call deadline is exceeded), bringing the
transport-grpctransport to parity with the HTTP transport.Problem
Over the
transport-grpctransport, a search whose client had already gone away kept running to completion server-side — we observed 17–30s of wasted execution after cancellation, consuming CPU, memory, and search-thread-pool capacity for a result no client would read. A client that times out and retries can leave multiple orphaned searches running, amplifying load when the cluster is already slow.Observed impact (production)
First caught during a segment merge — gRPC search requests spiked to >10s, driving up p99. Merges are just one trigger; this hurts during any period of elevated search latency (GC pauses, high load, expensive queries, cold caches, etc.). Whenever a search is slow enough that the client gives up (cancel, disconnect, or deadline), the abandoned request kept running across all shards, compounding the slowdown that made the client leave.
Representative slow log — a lookup over 256 shards spending 17.3s almost entirely in the query phase (
query=17318oftook_millis=17319), 0 hits:Since the time is spent almost entirely in the query phase, cancelling the coordinator task and its multi shard sub-tasks (what this change does) stops the work instead of letting it run the full 17.3s — relieving the p99 spike during merges and any other period of elevated latency.
Root cause
The HTTP path wraps the node client in
RestCancellableNodeClient, which captures the searchTaskviaNodeClient.executeLocally(...)and, onHttpChannelclose, issues aCancelTasksRequestfor it. The gRPC path (SearchServiceImpl.search) instead calledclient.search(searchRequest, listener), which discards the task, and never wired anything to gRPC's cancellation signal — so gRPC search had no cancellation-on-client-disconnect behavior at all.Fix
Mirror the HTTP design with a dedicated collaborator so the service handler stays focused and cancellation stays encapsulated:
GrpcCancellableNodeClient(new) — aFilterClientthat:NodeClient.executeLocally(...)to obtain theTask;ServerCallStreamObserver.setOnCancelHandler(...)to issue aCancelTasksRequestfor that task (as the tasks-origin system user, exactly likeRestCancellableNodeClient);isCancelled()check so a call cancelled betweenexecuteLocallyand handler registration is still cancelled.SearchServiceImpl— routes the search throughGrpcCancellableNodeClientwhen the call exposes a cancellation signal (the observer is aServerCallStreamObserverand the client is aNodeClient), and otherwise falls back to the existing path unchanged.Notes / scope
client.search(...)already routes throughexecuteLocallyinternally (discarding the task), so this only retains the task handle to enable cancellation.CancelTasksmachinery, which propagates the ban to shard-level child tasks on data nodes, so the actual query/fetch work stops — not just the coordinator.Files changed
.../transport/grpc/client/GrpcCancellableNodeClient.javaFilterClient.../transport/grpc/client/package-info.java.../transport/grpc/services/SearchServiceImpl.java.../transport/grpc/client/GrpcCancellableNodeClientTests.java.../transport/grpc/services/SearchServiceImplTests.java.../transport/grpc/SearchCancellationIT.javaRelated Issues
Resolves #22428
Testing
GrpcCancellableNodeClientTests: task is captured viaexecuteLocally; firing the cancel handler cancels the captured task; an already-cancelled call cancels immediately; a normally-completing call leaves the task running.SearchServiceImplTests: with aServerCallStreamObserver, the search is routed through the cancellable path (executeLocally+setOnCancelHandler, notclient.search); existing tests unchanged.SearchCancellationIT: boots the real gRPC transport, holds a search in the query phase, cancels the gRPC call via a cancellable context, and asserts aCancelTasksis issued for the search task. Fully event-driven (latches, noassertBusy/sleeps). Verified it fails with the fix disabled and passes with it enabled.grpcurl): on a node running the gRPC transport, ingest a large enough dataset that a search runs for several seconds (bulk-index many documents), then send an expensive gRPC search throughgrpcurlwith a short call deadline (-max-time) so the client abandons the call while the search is still executing. With the fix, the server-side task is cancelled as soon as the deadline expires — confirmed via the DEBUG cancel log and the tasks API showing the search taskcancelled=trueand then unwinding — instead of running to natural completion. Without the fix, the same request keeps running server-side for the query's full duration aftergrpcurlhas already returned.Run (automated):
Manual reproduction (local,
grpcurl):Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.