Skip to content

check for retention period to be a multiple of periodic table duration and relevant test #1564

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

Merged

Conversation

sandeepsukhani
Copy link
Contributor

@sandeepsukhani sandeepsukhani commented Aug 8, 2019

When the retention period is not a multiple of periodic table duration, we need to error out since Cortex doesn't yet support partial deletion of tables.
Added a check to validate retention against the latest config(assuming the last one is the latest) and relevant test.

Signed-off-by: Sandeep Sukhani [email protected]

@tomwilkie tomwilkie requested a review from gouthamve August 8, 2019 12:09
@bboreham
Copy link
Contributor

bboreham commented Aug 8, 2019

Rather than silently changing what the user asked for, we could just make this an error.

@sandeepsukhani sandeepsukhani force-pushed the retention-tables-count-fix branch 2 times, most recently from 433e25c to 2a43403 Compare August 9, 2019 10:51
@sandeepsukhani sandeepsukhani changed the title Retention should keep first periodic table even if its partially expired check for retention period to be a multiple of periodic table duration and relevant test Aug 9, 2019
…uration and relevant test

Signed-off-by: Sandeep Sukhani <[email protected]>
@sandeepsukhani sandeepsukhani force-pushed the retention-tables-count-fix branch from 28b2cfd to 4da3673 Compare August 14, 2019 11:45
Signed-off-by: Sandeep Sukhani <[email protected]>
@gouthamve gouthamve merged commit ab50f94 into cortexproject:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants