Skip to content

Properly handle error from ReadPartitionedGroupInfo in UpdatePartitionedGroupInfo - #7766

Open
alexqyle wants to merge 3 commits into
cortexproject:masterfrom
alexqyle:update-partitioned-group-fix
Open

Properly handle error from ReadPartitionedGroupInfo in UpdatePartitionedGroupInfo#7766
alexqyle wants to merge 3 commits into
cortexproject:masterfrom
alexqyle:update-partitioned-group-fix

Conversation

@alexqyle

@alexqyle alexqyle commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What this PR does:

Fixes a TOCTOU race condition in UpdatePartitionedGroupInfo where the error from ReadPartitionedGroupInfo was swallowed with _. When S3 returns a transient error (e.g. throttling with many concurrent compactors), existingPartitionedGroup would be nil, the existence guard would not fire, and the code would fall through to bkt.Upload() — overwriting an existing partitioned group file with a new CreationTime.

The fix captures the error and, if it is not ErrorPartitionedGroupInfoNotFound, logs it and returns — aborting creation so the group will be retried in the next compaction cycle.

Which issue(s) this PR fixes:
NA

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

…nedGroupInfo

Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Alex Le <leqiyue@amazon.com>
@alexqyle
alexqyle force-pushed the update-partitioned-group-fix branch from 15b31b9 to fca9531 Compare August 14, 2026 20:43
Signed-off-by: Alex Le <leqiyue@amazon.com>
@CharlieTLe
CharlieTLe requested review from a team and SungJin1212 and removed request for a team August 21, 2026 18:27

@SungJin1212 SungJin1212 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/compactor lgtm This PR has been approved by a maintainer size/L type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants