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. 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

Updated unit tests. make build runs successfully.

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?

comp tsdb.Compactor
}

var cancelFn = func() {}
Copy link
Member

Choose a reason for hiding this comment

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

Could we please not add this? 🙏 let's pass func() {} instead where needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I will fix this

@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?

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.

2 participants