Cleanup ES|QL T-Digest code duplication, add memory accounting by JonasKunz · Pull Request #143662 · elastic/elasticsearch

JonasKunz · 2026-03-05T08:00:35Z

removes the duplicate code around T-Digest to byte[] encoding/decoding
makes accesses to T-Digests stored in blocks allocation free by requiring a scratch input parameter (similar to how it works for BytesRefBlocks)
Adds the possibility to deep-copy T-Digests from blocks using a BreakingTDigestHolder with full memory accounting: This follows the pattern of BreakingBytesRefBuilder and will be used for the agg-state on first/last_over_time
Replaces the usages of TDigestState with TDigest: TDigestState is just a decorator with serialization which don't use and don't want to use

elasticsearchmachine · 2026-03-05T12:23:03Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

.../plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/mapper/EncodedTDigest.java

kkrik-es · 2026-03-05T13:32:27Z

...k/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/TDigestStates.java

-                blocks[offset] = blockFactory.newConstantTDigestBlock(resultHolder, 1);
-                blocks[offset + 1] = blockFactory.newConstantBooleanBlockWith(true, 1);
+                try (var tempHolder = BreakingTDigestHolder.create(breaker)) {
+                    tempHolder.set(merger, sum, min, max);


What happened to count?

I decided to remove it, because it is not needed and redundant. The count always is the sum of the counts of all centroids in the merged TDigest.

.../plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BreakingTDigestHolder.java

kkrik-es · 2026-03-05T13:36:30Z

No reason to include everything in a single PR. I'll get back to reviewing once you move BreakingBytesRefBuilder to a separate one. Agents can do that in minutes.

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/TDigestArrayBlock.java

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/TDigestHolder.java

kkrik-es · 2026-03-05T15:22:26Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/ResponseValueUtils.java

                return dateRangeToString(from, to);
            };
-            case TDIGEST -> (block, offset, scratch) -> ((TDigestBlock) block).getTDigestHolder(offset);
+            case TDIGEST -> (block, offset, scratch) -> ((TDigestBlock) block).getTDigestHolder(offset, new TDigestHolder());


Maybe wrap this in a helper that only takes offset as arg.

I'd prefer not to, as that would encourage the non-allocation free usage.
This method with the scratch parameter is a common pattern for block access, e.g. see BytesRefBlock:
https://github.com/elastic/elasticsearch/blob/main/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BytesRefBlock.java#L38

kkrik-es

Just a few minor comments, approving to get you going.

coderabbitai · 2026-03-06T08:06:50Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (2)

Team:Delivery
Team:Search - Inference

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: a7767d6a-2cdc-433c-a70d-f393d866ff06

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

…ic#143662)

…locations * upstream/main: (153 commits) ES|QL: Update docs for TOP_SNIPPETS and DECAY (elastic#143739) Correctly include endpoint id in log msg in AuthorizationPoller (elastic#143743) Bar searching or sorting on _seq_no when disabled (elastic#143600) Generalize `testClientCancellation` test (elastic#143586) JSON_EXTRACT: zero-copy byte slicing for object, array, and number extraction (elastic#143702) Track recycler pages in circuit breaker (elastic#143738) [ESQL] Enable distributed pipeline breakers for external sources via FragmentExec (elastic#143696) Adding 'mode' and 'codec' fields to ES monitoring template (elastic#143673) [ESQL] Columnar I/O and vectorized block conversion for external sources (elastic#143703) Fix flaky MMR diversification YAML tests (elastic#143706) ES|QL codegen: check builder arguments for vector support (elastic#143724) Add Views Security Model (elastic#141050) ESQL: Prevent pushdown of unmapped fields in filters and sorts (elastic#143460) Don't run seq_no pruning tests in release CI (elastic#143725) ESQL: Support intra-row field references in ROW command (elastic#140217) ES|QL: Remove implicit limit in FORK branches in CSV tests (elastic#143601) IndexRoutingTests with and without synthetic id (elastic#143566) Synthetic id upgrade test in serverless (elastic#142471) Disable "Review skipped" comments for PRs without specified labels (elastic#143728) Cleanup ES|QL T-Digest code duplication, add memory accounting (elastic#143662) ...

…ic#143662)

JonasKunz added 3 commits March 5, 2026 08:59

Add memory tracking and avoid allocations for ES|QL TDigest impl

a67a76e

Update tests

30faf9f

Fix merge rebase conflicts

e62e4b0

JonasKunz added >non-issue :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL labels Mar 5, 2026

elasticsearchmachine added v9.4.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Mar 5, 2026

JonasKunz and others added 7 commits March 5, 2026 09:21

Fix javadoc

b28ed9f

Fix to-string floating point inaccuracies

d3e8ca7

Merge branch 'main' into esql-tdigest-cleanup

1bc4f81

Prevent mutation of empty TDigestHolder

80c23f4

Add tests for BreakingTDigestHolder

4f41f8a

Rename bytes ref

9d07838

[CI] Auto commit changes from spotless

2635733

JonasKunz changed the title ~~Cleanup ES|QL T-Digest code duplication, reduce allocations~~ Cleanup ES|QL T-Digest code duplication, add memory accounting Mar 5, 2026

JonasKunz marked this pull request as ready for review March 5, 2026 12:22

JonasKunz requested a review from kkrik-es March 5, 2026 12:22

elasticsearchmachine added the Team:StorageEngine label Mar 5, 2026

kkrik-es reviewed Mar 5, 2026

View reviewed changes

.../plugin/core/src/main/java/org/elasticsearch/xpack/core/analytics/mapper/EncodedTDigest.java Show resolved Hide resolved

kkrik-es reviewed Mar 5, 2026

View reviewed changes

.../plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/BreakingTDigestHolder.java Outdated Show resolved Hide resolved

JonasKunz added 2 commits March 5, 2026 15:35

Remove BreakingTDigestHolder from PR

2ff9473

Add assertions to iterators

52ed2f8

JonasKunz requested a review from kkrik-es March 5, 2026 14:43

[CI] Auto commit changes from spotless

9544287

kkrik-es reviewed Mar 5, 2026

View reviewed changes

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/TDigestArrayBlock.java Show resolved Hide resolved

kkrik-es reviewed Mar 5, 2026

View reviewed changes

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/TDigestHolder.java Show resolved Hide resolved

kkrik-es reviewed Mar 5, 2026

View reviewed changes

kkrik-es approved these changes Mar 5, 2026

View reviewed changes

Update javadoc

6bd58cd

Merge branch 'main' into esql-tdigest-cleanup

c2c8b8a

JonasKunz enabled auto-merge (squash) March 6, 2026 08:13

JonasKunz merged commit cea9eb7 into elastic:main Mar 6, 2026
36 checks passed

JonasKunz deleted the esql-tdigest-cleanup branch March 6, 2026 09:45

spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Mar 6, 2026

Cleanup ES|QL T-Digest code duplication, add memory accounting (elast…

de81bbf

…ic#143662)

prwhelan mentioned this pull request Mar 6, 2026

[ML] Wait for cluster state in test #143767

Merged

sidosera pushed a commit to sidosera/elasticsearch that referenced this pull request Mar 6, 2026

Cleanup ES|QL T-Digest code duplication, add memory accounting (elast…

77d422a

…ic#143662)

prwhelan mentioned this pull request Mar 9, 2026

[Transform] Disable PIT for CPS #143876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup ES|QL T-Digest code duplication, add memory accounting#143662

Cleanup ES|QL T-Digest code duplication, add memory accounting#143662
JonasKunz merged 15 commits intoelastic:mainfrom
JonasKunz:esql-tdigest-cleanup

JonasKunz commented Mar 5, 2026 •

edited

Loading

Uh oh!

elasticsearchmachine commented Mar 5, 2026

Uh oh!

Uh oh!

kkrik-es Mar 5, 2026

Uh oh!

JonasKunz Mar 5, 2026

Uh oh!

Uh oh!

kkrik-es commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

kkrik-es Mar 5, 2026

Uh oh!

JonasKunz Mar 6, 2026

Uh oh!

kkrik-es left a comment

Uh oh!

coderabbitai bot commented Mar 6, 2026 •

edited

Loading

Review skipped

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

JonasKunz commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Mar 5, 2026

Uh oh!

Uh oh!

kkrik-es Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

JonasKunz Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kkrik-es commented Mar 5, 2026

Uh oh!

Uh oh!

Uh oh!

kkrik-es Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

JonasKunz Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

kkrik-es left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JonasKunz commented Mar 5, 2026 •

edited

Loading

coderabbitai bot commented Mar 6, 2026 •

edited

Loading