Querying Performance Degradation on Realtime Tasks

[GWphua]

Affected Version

Since v32

Description

We are deploying our clusters using the MM-less Kubernetes extension. Recently, as we are upgrading our clusters from v27 to v37, we experienced a query degradation in segmentMetadata queries. We have yet to establish whether other query types are affected too.

Peon Querying Performance

• After upgrading to v37, performance degradation found when querying on realtime tasks.
  • Currently, the cause is found on segmentMetadata queries, and the benchmark features timeseries queries... It might be possible for this issue to affect performance on other query types.
  • Identified cause to be Merge hydrant runners flatly for realtime queries. #15757, created to address memory load when serving GroupBy queries.
  • While Restore Sink Metric Emission Behaviour: Emit them per-Sink instead of per-FireHydrant #17170 tried to mitigate the issue, the performance is still sub-optimal.

The following benchmark is done locally on SinkQuerySegmentWalkerBenchmark, measuing ms/op for differing number of hydrants per sink.

	10	50	100	200
v27	0.089	0.162	0.265	0.493
before-pr-15757	0.089	0.165	0.274	0.479
after-pr-15757-before-pr-17170	0.519	2.351	4.847	9.112
after-pr-17170	0.231	0.784	1.443	2.791
v37	0.228	0.825	1.499	2.930

Additionally, there still appears to be a performance regression between #17170, which was included in v32, and v37. I have not yet identified the underlying cause, but this does suggest that we may benefit from introducing more systematic benchmarking for future PRs that could affect query or ingestion performance. This would help catch regressions earlier and provide a clearer basis for evaluating performance-sensitive changes.

[GWphua]

Throwing some suggestions here just to keep the discussion going:

1. Inspiration from Add setProcessingThreadNames context parameter. #12514: Turn off per-hydrant thread renaming in the realtime path by default.
   SpecificSegmentQueryRunner renames the processing thread by default via setProcessingThreadNames=true at SpecificSegmentQueryRunner.java. After Merge hydrant runners flatly for realtime queries. #15757, this happens once per hydrant in SinkQuerySegmentWalker.java (line 311). Add a constructor/default flag so the Peon hydrant path defaults this to false, while preserving existing behavior elsewhere.

2. Use flat hydrant merging only for query types that need it.
   Merge hydrant runners flatly for realtime queries. #15757 flattened non-bySegment hydrants at SinkQuerySegmentWalker.java, mainly to help GroupBy memory behavior. For segmentMetadata, the flat shape routes many cheap hydrant runners through SegmentMetadataQueryRunnerFactory.mergeRunners, which is a ChainedExecutionQueryRunner at SegmentMetadataQueryRunnerFactory.java. A fix could keep the flat path for GroupBy, but restore per-sink/nested merging for segmentMetadata and maybe other cheap metadata-style queries.

3. Collapse the per-hydrant wrapper stack.
   Each hydrant currently gets multiple wrappers: segment metrics, cache/wait metrics, CPU metrics, and SpecificSegmentQueryRunner at SinkQuerySegmentWalker.java. Restore Sink Metric Emission Behaviour: Emit them per-Sink instead of per-FireHydrant #17170 restored per-sink metric emission, but implementation still pays per-hydrant wrapper overhead. A better implementation could merge metrics/CPU/specific-segment behavior into fewer wrappers or move metrics accumulation around the per-sink merged runner.

[jtuglu1]

but this does suggest that we may benefit from introducing more systematic benchmarking for future PRs that could affect query or ingestion performance.

+1 here. I have spoken to @clintropolis about this before. IMO the current problem with the benchmarks/ is that there is no standard format/max runtime to the benchmarks, which makes it hard to run automatically. Do you think you could put together a format for benchmarks that we could run on every PR? Ideally, these would be microbenchmarks (e.g < XXs total runtime).

If there is a standard for writing a benchmark file, we can use the results from the benchmarks to check if there are any statistically significant regressions worth recording/flagging. One particular one that has caught my eye is: https://github.com/apps/codspeed-hq, they are apparently free for OSS projects (it is used in other ASF projects AFAIK).

cc @gianm

[GWphua]

@jtuglu1 Thanks for the fast turnaround. A standard benchmark format with bounded runtime would be useful, and CodSpeed looks worth exploring.

Unfortunately, I don’t think I’ll have the bandwidth to take this on right now. I’m happy to share context from the benchmark work I’ve done, but I don’t want to overcommit on something I won’t be able to drive properly. Maybe we can set this up as a separate issue for pickup.