Skip to content

Fix interpolation for data points at bucket boundaries#139798

Merged
kkrik-es merged 12 commits intoelastic:mainfrom
kkrik-es:fix/139732
Dec 19, 2025
Merged

Fix interpolation for data points at bucket boundaries#139798
kkrik-es merged 12 commits intoelastic:mainfrom
kkrik-es:fix/139732

Conversation

@kkrik-es
Copy link
Contributor

@kkrik-es kkrik-es commented Dec 19, 2025

Interpolation logic didn't cover the case where there's a single data point at the lower boundary and just a previous state, or a single point at the upper boundary and just a next state. If this happens, the data point matches the only interpolated value, so there are no 2 data points within the state to calculate the delta. The fix is to use the delta from the closest data point in the adjacent state.

While there, added the following fixes to the randomized testing suite:

  • Take adjacent buckets into account only when calculating counter deltas across boundaries.
  • Fix delta calculation across boundaries when first and last value are the same (0 delta).

Fixes #139732

@kkrik-es kkrik-es self-assigned this Dec 19, 2025
@kkrik-es kkrik-es added >bug auto-backport Automatically create backport pull requests when merged Team:StorageEngine :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL v9.3.1 labels Dec 19, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @kkrik-es, I've created a changelog YAML for you.

11.13218 | 3339.65266 | 2024-05-10T00:10:00.000Z | prod

;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Repeated above.

@kkrik-es kkrik-es requested a review from martijnvg December 19, 2025 12:57
@kkrik-es kkrik-es marked this pull request as ready for review December 19, 2025 12:57
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 . I only left questions.

if (lastTsSec == firstTsSec) {
// Check for the case where there is only one sample in state, right at the boundary towards a non-empty adjacent state.
if (state.samples == 1) {
if (previousState != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is is possible that both previousState and nextState is null or that they both are not null?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case is this allowed and then maybe do else if block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are both null, we just return Nan below - can't do better for now.
If they are both not null, we'd get a value to the other boundary so lastTsSec == firstTsSec would be false. I'll add asserts for this.

* Rate and increase calculations use interpolation at the boundaries between time buckets
*/
RATE_WITH_INTERPOLATION,
RATE_WITH_INTERPOLATION_V2,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just to make bwc tests pass, right? Cause once this is merged and back ported, the no mode will report RATE_WITH_INTERPOLATION?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct for bwc. I didn't use the new tag on unaffected tests, but can do so if you prefer.

pr: 139798
summary: Fix interpolation for data points at bucket boundaries
area: ES|QL
type: bug
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make this issue an non-issue, given that this bug isn't released?

@github-actions
Copy link
Contributor

ℹ️ Important: Docs version tagging

👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version.

We use applies_to tags to mark version-specific features and changes.

Expand for a quick overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

@kkrik-es kkrik-es enabled auto-merge (squash) December 19, 2025 16:42
@kkrik-es kkrik-es merged commit b6ead64 into elastic:main Dec 19, 2025
35 checks passed
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
9.3 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 139798

@kkrik-es
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
9.3

Questions ?

Please refer to the Backport tool documentation

kkrik-es added a commit to kkrik-es/elasticsearch that referenced this pull request Dec 19, 2025
* Fix flakiness in TSDataGenerationHelper

* Fix interpolation for data points at bucket boundaries

* indentation

* Update docs/changelog/139798.yaml

* [CI] Auto commit changes from spotless

* fix csv tests

* asserts and date factor

* update doc

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
(cherry picked from commit b6ead64)

# Conflicts:
#	muted-tests.yml
elasticsearchmachine pushed a commit that referenced this pull request Dec 22, 2025
#139850)

* Fix interpolation for data points at bucket boundaries (#139798)

* Fix flakiness in TSDataGenerationHelper

* Fix interpolation for data points at bucket boundaries

* indentation

* Update docs/changelog/139798.yaml

* [CI] Auto commit changes from spotless

* fix csv tests

* asserts and date factor

* update doc

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
(cherry picked from commit b6ead64)

# Conflicts:
#	muted-tests.yml

* Add muted test for RandomizedRollingUpgradeIT
@kkrik-es kkrik-es deleted the fix/139732 branch December 22, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >bug :StorageEngine/ES|QL Timeseries / metrics / PromQL / logsdb capabilities in ES|QL Team:StorageEngine v9.3.1 v9.4.0

3 participants