Merged
Conversation
800ee11 to
41dcfd1
Compare
42d1175 to
4bc6ea2
Compare
81d2883 to
5a07957
Compare
f141f16 to
7c758db
Compare
7c758db to
1b4cd60
Compare
Collaborator
|
Hi @dnhatn, I've created a changelog YAML for you. |
dnhatn
commented
Jan 21, 2026
x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec
Show resolved
Hide resolved
dnhatn
commented
Jan 21, 2026
...va/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PartitionTimeSeriesTests.java
Show resolved
Hide resolved
kkrik-es
reviewed
Jan 21, 2026
| ); | ||
|
|
||
| public static final Setting<ByteSizeValue> RATE_BUFFER_SIZE = new Setting<>("esql.rate_buffer_size", settings -> { | ||
| long oneThird = JvmInfo.jvmInfo().getMem().getHeapMax().getBytes() / 2; |
Contributor
There was a problem hiding this comment.
Should this be less aggressive, e.g. 1/10? 2 queries will trip the circuit breaker, or I misread this?
Member
Author
There was a problem hiding this comment.
Yes, I updated the estimate and reduced this to 1/10.
kkrik-es
reviewed
Jan 21, 2026
| } | ||
| String filter2 = ""; | ||
| if (queryEndTs != null || randomBoolean()) { | ||
| filter2 = "| WHERE @timestamp <= \"" + (queryEndTs != null ? queryEndTs : maxTimestampFromData) + "\""; |
Contributor
There was a problem hiding this comment.
TRANGE works too? Maybe add a test for it.
kkrik-es
approved these changes
Jan 21, 2026
Contributor
kkrik-es
left a comment
There was a problem hiding this comment.
This is very promising! Some minor comments and suggestions.
Contributor
|
++ this looks like the right tradeoff. The performance for |
Collaborator
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
Member
Author
|
Thanks Kostas! |
szybia
added a commit
to szybia/elasticsearch
that referenced
this pull request
Jan 21, 2026
…-tests * upstream/main: (104 commits) Partition time-series source (elastic#140475) Mute org.elasticsearch.xpack.esql.heap_attack.HeapAttackSubqueryIT testManyRandomKeywordFieldsInSubqueryIntermediateResultsWithSortManyFields elastic#141083 Reindex relocation: skip nodes marked for shutdown (elastic#141044) Make fails on fixture caching not fail image building (elastic#140959) Add multi-project tests for get and list reindex (elastic#140980) Painless docs overhaul (reference) (elastic#137211) Panama vector implementation of codePointCount (elastic#140693) Enable PromQL in release builds (elastic#140808) Update rest-api-spec for Jina embedding task (elastic#140696) [CI] ShardSearchPhaseAPMMetricsTests testUniformCanMatchMetricAttributesWhenPlentyOfDocumentsInIndex failed (elastic#140848) Combine hash computation with bloom filter writes/reads (elastic#140969) Refactor posting iterators to provide more information (elastic#141058) Wait for cluster to recover to yellow before checking index health (elastic#141057) (elastic#141065) Fix repo analysis read count assertions (elastic#140994) Fixed a bug in logsdb rolling upgrade sereverless tests involving par… (elastic#141022) Fix readiness edge case on startup (elastic#140791) PromQL: fix quantile function (elastic#141033) ignore `mmr` command for check (in development) (elastic#140981) Use Double.compare to compare doubles in tdigest.Sort (elastic#141049) Migrate third party module tests using legacy test clusters framework (elastic#140991) ...
Member
Author
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
dnhatn
added a commit
to dnhatn/elasticsearch
that referenced
this pull request
Jan 21, 2026
Today, for rate aggregations, we can't partition a target shard by DOC or SEGMENT, only by SHARD. Because of this, we rely on ROUND_TO from time_bucket to partition the target shard into multiple slices. However, the number of slices can be either too many or too few. Too few large slices might under-utilize the CPUs and require a large amount of memory to buffer data points. Many smaller slices solve these issues but add significant overhead. This change replaces the ROUND_TO partitioning with time-based partitioning for time-series sources. One potential overhead of this method compared to ROUND_TO is that, with ROUND_TO, we do not need to perform round_to for the timestamp, as the query source provides this constant. (cherry picked from commit 54ee110) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
dnhatn
added a commit
that referenced
this pull request
Jan 22, 2026
Today, for rate aggregations, we can't partition a target shard by DOC or SEGMENT, only by SHARD. Because of this, we rely on ROUND_TO from time_bucket to partition the target shard into multiple slices. However, the number of slices can be either too many or too few. Too few large slices might under-utilize the CPUs and require a large amount of memory to buffer data points. Many smaller slices solve these issues but add significant overhead. This change replaces the ROUND_TO partitioning with time-based partitioning for time-series sources. One potential overhead of this method compared to ROUND_TO is that, with ROUND_TO, we do not need to perform round_to for the timestamp, as the query source provides this constant. (cherry picked from commit 54ee110) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
kkrik-es
reviewed
Jan 22, 2026
| ); | ||
|
|
||
| String filter = randomFrom( | ||
| "\"2023-11-20T12:16:03.360Z\" <= @timestamp AND @timestamp <= \"2023-11-20T12:57:02.250Z\"", |
dnhatn
added a commit
to dnhatn/elasticsearch
that referenced
this pull request
Jan 23, 2026
This reverts commit 54ee110.
dnhatn
added a commit
to dnhatn/elasticsearch
that referenced
this pull request
Jan 24, 2026
This reverts commit 54ee110.
dnhatn
added a commit
that referenced
this pull request
Jan 24, 2026
The competitive benchmark shows a significant performance regression from #140475. This is likely due to the high number of CPUs in the benchmark, which results in many small slices and adds significant overhead. It is better to revert this change for now and adjust the partitioning for cases with high CPU and more than one shard before merging again. Relates #14047
dnhatn
added a commit
to dnhatn/elasticsearch
that referenced
this pull request
Jan 24, 2026
The competitive benchmark shows a significant performance regression from elastic#140475. This is likely due to the high number of CPUs in the benchmark, which results in many small slices and adds significant overhead. It is better to revert this change for now and adjust the partitioning for cases with high CPU and more than one shard before merging again. Relates elastic#14047 (cherry picked from commit 1146b88) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jan 24, 2026
The competitive benchmark shows a significant performance regression from #140475. This is likely due to the high number of CPUs in the benchmark, which results in many small slices and adds significant overhead. It is better to revert this change for now and adjust the partitioning for cases with high CPU and more than one shard before merging again. Relates #14047 (cherry picked from commit 1146b88) # Conflicts: # x-pack/plugin/esql/qa/testFixtures/src/main/resources/k8s-timeseries.csv-spec
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today, for rate aggregations, we can't partition a target shard by DOC or SEGMENT, only by SHARD. Because of this, we rely on ROUND_TO from time_bucket to partition the target shard into multiple slices. However, the number of slices can be either too many or too few. Too few large slices might under-utilize the CPUs and require a large amount of memory to buffer data points. Many smaller slices solve these issues but add significant overhead.
This change replaces the ROUND_TO partitioning with time-based partitioning for time-series sources. One potential overhead of this method compared to ROUND_TO is that, with ROUND_TO, we do not need to perform round_to for the timestamp, as the query source provides this constant.
I benchmarked this change with:
Although there is a performance regression for 5m queries, I think we should proceed with this change, as overall it should significantly speed up queries and manage memory better. I will look into reducing the overhead of round_to for time-series.
Related to #139186