Skip to content

Conversation

charleskorn
Copy link
Contributor

What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a state where the head of the buffer is at the end of the slice, and the tail was at the beginning of the slice. In this case, calling Remove for the item at the end of the slice would return the correct value, as would the subsequent call for the item at the beginning of the slice, but all following calls would return incorrect values.

Which issue(s) this PR fixes or relates to

#11189

Checklist

  • Tests updated.
  • [n/a] Documentation added.
  • [covered by Mimir Query Engine #10067] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • [n/a] about-versioning.md updated with experimental features.

@charleskorn charleskorn added the changelog-not-needed PRs that don't need a CHANGELOG.md entry label Jul 6, 2025
@charleskorn charleskorn marked this pull request as ready for review July 6, 2025 23:54
@charleskorn charleskorn requested a review from a team as a code owner July 6, 2025 23:54
@charleskorn charleskorn enabled auto-merge (squash) July 6, 2025 23:57
@charleskorn charleskorn merged commit 8c9ffff into main Jul 7, 2025
48 checks passed
@charleskorn charleskorn deleted the charleskorn/cse-fix branch July 7, 2025 01:14
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
mimir-github-bot bot pushed a commit that referenced this pull request Jul 7, 2025
… incorrect results (#11989)

#### What this PR does

This PR fixes an issue where common subexpression elimination (CSE) can
cause a query to return incorrect query results.

The issue would occur if the ring buffer used to buffer data was in a
state where the head of the buffer is at the end of the slice, and the
tail was at the beginning of the slice. In this case, calling `Remove`
for the item at the end of the slice would return the correct value, as
would the subsequent call for the item at the beginning of the slice,
but all following calls would return incorrect values.

#### Which issue(s) this PR fixes or relates to

#11189

#### Checklist

- [x] Tests updated.
- [n/a] Documentation added.
- [covered by #10067] `CHANGELOG.md` updated - the order of entries
should be `[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If
changelog entry is not needed, please add the `changelog-not-needed`
label to the PR.
- [n/a]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

(cherry picked from commit 8c9ffff)
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11993)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11994)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11995)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11990)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11991)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
charleskorn added a commit that referenced this pull request Jul 7, 2025
… return incorrect results (#11992)

Backport 8c9ffff from #11989

Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com>
@mimir-github-bot
Copy link
Contributor

The backport to release-2.17 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-11989-to-release-2.17 origin/release-2.17
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 8c9fffff376a6433f9d5713cdf88ee757ab4397e
# Push it to GitHub
git push --set-upstream origin backport-11989-to-release-2.17
git switch main
# Remove the local backport branch
git branch -D backport-11989-to-release-2.17

Then, create a pull request where the base branch is release-2.17 and the compare/head branch is backport-11989-to-release-2.17.

@charleskorn
Copy link
Contributor Author

Backport to 2.17 isn't necessary, as the release is based on the r350 branch and this fix was already backported there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants