Skip to content

compactv2: propagate series set error in RelabelModifier - #8937

Open
aeron-gh wants to merge 1 commit into
thanos-io:mainfrom
aeron-gh:fix-relabel-modifier-set-err
Open

compactv2: propagate series set error in RelabelModifier#8937
aeron-gh wants to merge 1 commit into
thanos-io:mainfrom
aeron-gh:fix-relabel-modifier-set-err

Conversation

@aeron-gh

@aeron-gh aeron-gh commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

thanos tools bucket rewrite --rewrite.to-relabel-config can write a block that is missing data and report success while doing it.

RelabelModifier.Modify drains the source ChunkSeriesSet into maps and builds a new listChunkSeriesSet from what it collected, but it never checks set.Err(). Next() returns false on failure as well as on completion, so if reading the source block fails partway through, the loop simply ends and Modify returns a normal looking result holding only the series it read before the failure. The caller then writes that out as the new block.

Three things in the same package suggest this is an oversight rather than intentional:

  • the same function already checks the inner chunk iterator in two places, and returns errorOnlyStringIter when it fails
  • DeletionModifier propagates the source error through delModifierSeriesSet.Err(), which forwards d.ChunkSeriesSet.Err(). RelabelModifier cannot rely on that because it returns a brand new set, so it has to check explicitly
  • the dry run path in compactor.go does check set.Err(), so a dry run surfaces the failure today while the actual rewrite does not

Fixes #8933

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • pkg/compactv2/modifiers.go: return the series set error from RelabelModifier.Modify, reusing the errorOnlyStringIter path the function already uses for chunk iterator errors.

Verification

go build passes on pkg/compactv2. go vet reports only two pre-existing stdmethods warnings on lines this change does not touch.

@aeron-gh aeron-gh closed this Jul 20, 2026
@aeron-gh aeron-gh reopened this Jul 20, 2026
@aeron-gh

Copy link
Copy Markdown
Contributor Author

@fpetkovski
gentle ping on this one, when u have some time
thanks !

@aeron-gh
aeron-gh force-pushed the fix-relabel-modifier-set-err branch from 7221949 to 96a687a Compare July 27, 2026 18:42
@aeron-gh aeron-gh closed this Jul 27, 2026
@aeron-gh aeron-gh reopened this Jul 27, 2026
@aeron-gh

Copy link
Copy Markdown
Contributor Author

@GiedriusS
ci is green, bump 🙂 No rush just flagging it whenever a reviewer can take a look. Thanks!

@GiedriusS

Copy link
Copy Markdown
Member

Hi, there's no rush. Please be patient.

Signed-off-by: aeron-gh <agab0323@gmail.com>
@aeron-gh
aeron-gh force-pushed the fix-relabel-modifier-set-err branch from 96a687a to 1ae7847 Compare August 14, 2026 08:55
@aeron-gh

Copy link
Copy Markdown
Contributor Author

Hi, there's no rush. Please be patient.

@GiedriusS
Understood, sorry for the noise. I'll leave it with you.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.50%. Comparing base (d567cbc) to head (1ae7847).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/compactv2/modifiers.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8937   +/-   ##
=======================================
  Coverage   64.50%   64.50%           
=======================================
  Files         289      289           
  Lines       37352    37354    +2     
=======================================
+ Hits        24094    24096    +2     
+ Misses      11160    11158    -2     
- Partials     2098     2100    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compactv2: RelabelModifier ignores series set error and can write partial blocks

2 participants