-
Notifications
You must be signed in to change notification settings - Fork 817
make delete request cancellation duration configurable and rename data-purger to purger #2760
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make delete request cancellation duration configurable and rename data-purger to purger #2760
Conversation
…a-purger to purger Signed-off-by: Sandeep Sukhani <[email protected]>
Signed-off-by: Sandeep Sukhani <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but I agree the name should be shorter. Will think of something soon and update the thread.
Signed-off-by: Sandeep Sukhani <[email protected]>
@@ -50,6 +50,7 @@ | |||
* [CHANGE] Available command-line flags are printed to stdout, and only when requested via `-help`. Using invalid flag no longer causes printing of all available flags. #2691 | |||
* [CHANGE] Experimental Memberlist ring: randomize gossip node names to avoid conflicts when running multiple clients on the same host, or reusing host names (eg. pods in statefulset). Node name randomization can be disabled by using `-memberlist.randomize-node-name=false`. #2715 | |||
* [CHANGE] Memberlist KV client is no longer considered experimental. #2725 | |||
* [CHANGE] Change target flag for purger from `data-purger` to `purger` and make delete request cancellation duration configurable. #2760 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify where this happened?
Not, for instance, here: https://github.com/cortexproject/cortex/pull/2760/files#diff-596d5408728e4813da55bfbc5a171d3dR63
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad. I just renamed the components and missed changing the flag. Will open a PR to fix this.
Did we mark the purger as experimental? If not, changing the name would trigger a major release. |
Yes, it is marked experimental. See https://github.com/cortexproject/cortex/blob/master/pkg/chunk/purger/purger.go#L141 |
What this PR does:
Duration until which delete requests are allowed to be cancelled after creation is made configurable. This would help with integration tests and run this initially with a higher value until we have more confidence in its stability.
This PR also renames flags and references from datapurger to purger for simplicity and consistency.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]