Skip to content

Conversation

@anna-tran
Copy link
Contributor

@anna-tran anna-tran commented Dec 4, 2025

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

Changes

This PR creates a new context for each compaction worker and also passes the cancel function for that worker's context. It introduces a new monitoring function in the lifecycle callback that allows implementations of the lifecycle to cancel the context if there is any issue with the compaction plan/process while the compaction is underway.

This allows a single compaction to be aborted/failed in case of an issue in the planning phase, and only affect that single compaction job.

The change will help to address this issue in Cortex: cortexproject/cortex#7135

Verification

make build runs successfully.

anna-tran added a commit to anna-tran/thanos that referenced this pull request Dec 4, 2025
anna-tran added a commit to anna-tran/thanos that referenced this pull request Dec 4, 2025
@anna-tran anna-tran force-pushed the unique-worker-context branch from 0782fdf to 3030034 Compare December 4, 2025 18:40
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Why cannot whoever calls Plan() cancel the context when they need to cancel it?

@anna-tran
Copy link
Contributor Author

anna-tran commented Dec 8, 2025

Why cannot whoever calls Plan() cancel the context when they need to cancel it?

This is also an option. We could make a new error chan for each worker (or reuse the existing one) in the compact method and spin up a new goroutine which waits to see if any error is received. If received then it can cancel the context, allowing the compaction to abort. What do you think?

anna-tran added a commit to anna-tran/thanos that referenced this pull request Dec 12, 2025
@anna-tran anna-tran force-pushed the unique-worker-context branch from bc1ed66 to 6543a18 Compare December 12, 2025 01:20
@pull-request-size pull-request-size bot added size/S and removed size/M labels Dec 12, 2025
Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

Could you show changes on the Cortex side? I don't get why we need to pass the cancel function.

@anna-tran
Copy link
Contributor Author

anna-tran commented Dec 17, 2025

I added a comment regarding how this would work in Cortex, and will update that with proposed code changes.
cortexproject/cortex#7135 (comment)

@anna-tran anna-tran force-pushed the unique-worker-context branch from 6543a18 to 7591b1a Compare December 22, 2025 22:47
@anna-tran anna-tran force-pushed the unique-worker-context branch from 7591b1a to f9c9564 Compare January 7, 2026 21:47
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.

3 participants