Skip to content

feat(query): per-dispatch latency metric tagged by local vs remote - #2273

Open
siri-varma wants to merge 1 commit into
developfrom
users/svegiraju/query-dispatch-latency-metric
Open

feat(query): per-dispatch latency metric tagged by local vs remote#2273
siri-varma wants to merge 1 commit into
developfrom
users/svegiraju/query-dispatch-latency-metric

Conversation

@siri-varma

Copy link
Copy Markdown

What

Adds a single dispatch-latency metric measured at the PlanDispatcher itself, tagged by whether the dispatch is local (in-process) or remote (actor / grpc / flight).

Exported to Prometheus as query_dispatch_latency_seconds{dispatch="local"|"remote", dataset=...}.

Why

We wanted to compare query dispatch latency across transport paths. Measuring at QueryPlanner.dispatchExecPlan is leaky — some code calls execPlan.dispatcher.dispatch(...) directly (e.g. PromQLGrpcServer.executePlan, the Flight producer, and internal child-plan dispatch), bypassing it. The dispatcher is the single choke point that every dispatch flows through, so the measurement belongs there.

How

  • PlanDispatcher.dispatch / dispatchStreaming are now final template methods that record latency and delegate the actual work to abstract doDispatch / doDispatchStreaming.
  • The metric is labeled via the existing PlanDispatcher.isLocalCall (dispatch=local|remote) plus dataset.
  • All implementations — InProcessPlanDispatcher, ActorPlanDispatcher, RemoteActorPlanDispatcher, GrpcPlanDispatcher, FlightPlanDispatcher — and the test dummy dispatchers implement doDispatch / doDispatchStreaming instead of dispatch / dispatchStreaming.
  • RemoteActorPlanDispatcher delegates to the inner dispatcher's doDispatch to avoid double-counting its wrapped dispatch.

Notes

  • The local/remote distinction is a dispatcher-intrinsic property (isLocalCall); no field is added to QueryContext / proto, so there are no cross-node (Kryo/proto) serialization concerns.
  • Measurement is per-dispatch, so the histogram captures every dispatch (top-level and child), broken down by local/remote.

🤖 Generated with Claude Code

Measure ExecPlan dispatch latency at the PlanDispatcher itself -- the single
choke point every dispatch flows through -- rather than wrapping individual
call sites, which were inconsistent (some code calls dispatcher.dispatch
directly, bypassing QueryPlanner.dispatchExecPlan).

- PlanDispatcher.dispatch/dispatchStreaming are now final template methods
  that time the dispatch and record "query-dispatch-latency", delegating the
  actual work to doDispatch/doDispatchStreaming.
- Tagged "dispatch" (local|remote, derived from isLocalCall) and "dataset";
  exported to Prometheus as query_dispatch_latency_seconds.
- Implementations (InProcess/Actor/RemoteActor/Grpc/Flight + test dummies)
  rename dispatch -> doDispatch and dispatchStreaming -> doDispatchStreaming.
- RemoteActorPlanDispatcher delegates via the inner doDispatch to avoid
  double-counting its wrapped dispatch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant