-
Notifications
You must be signed in to change notification settings - Fork 795
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
Implementing Bucket index sync status file #5446
Implementing Bucket index sync status file #5446
Conversation
d8b1794
to
a4334c5
Compare
Signed-off-by: Alan Protasio <[email protected]>
a4334c5
to
8490085
Compare
Signed-off-by: Alan Protasio <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>
Signed-off-by: Alan Protasio <[email protected]>
idxs.Status = bucketindex.CustomerManagedKeyError | ||
// Making the tenant non queryable until 3x the cleanup interval to give time to compactors and storegateways | ||
// to reload the bucket index in case the key access is re-granted | ||
idxs.NonQueryableUntil = time.Now().Add(3 * c.cfg.CleanupInterval).Unix() |
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.
Nit: can we extract the magic number 3 to some meaningfully name constant?
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.
done
if idxs, err := bucketindex.ReadSyncStatus(ctx, i.TSDBState.bucket, userID, logutil.WithContext(ctx, i.logger)); err == nil { | ||
// Skip blocks shipping if the bucket index failed to sync due to CMK errors. | ||
if idxs.Status == bucketindex.CustomerManagedKeyError { | ||
level.Info(logutil.WithContext(ctx, i.logger)).Log("msg", "skipping shipping blocks due CustomerManagedKeyError", "user", userID) |
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.
Should this be a warn
rather than info
?
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.
done
looks good to me, thanks! |
* Implementing Bucket index sync status Signed-off-by: Alan Protasio <[email protected]> * fixing bug when returning from cache Signed-off-by: Alan Protasio <[email protected]> * Addressing some comments Signed-off-by: Alan Protasio <[email protected]> * Changelog Signed-off-by: Alan Protasio <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Yijie Qin <[email protected]>
* Implementing Bucket index sync status file (#5446) * Implementing Bucket index sync status Signed-off-by: Alan Protasio <[email protected]> * fixing bug when returning from cache Signed-off-by: Alan Protasio <[email protected]> * Addressing some comments Signed-off-by: Alan Protasio <[email protected]> * Changelog Signed-off-by: Alan Protasio <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Yijie Qin <[email protected]> * Implementing multi level index cache (#5451) Signed-off-by: Yijie Qin <[email protected]> * remove the user from am state replication key Signed-off-by: Yijie Qin <[email protected]> * expose the key label Signed-off-by: Yijie Qin <[email protected]> * add Changelog Signed-off-by: Yijie Qin <[email protected]> * fix comments Signed-off-by: Yijie Qin <[email protected]> * change to use type Signed-off-by: Yijie Qin <[email protected]> * address comment Signed-off-by: Yijie Qin <[email protected]> * fix comment Signed-off-by: Yijie Qin <[email protected]> * use type instead of key Signed-off-by: Yijie Qin <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Yijie Qin <[email protected]> Co-authored-by: Alan Protasio <[email protected]>
What this PR does:
This PR introduce a new file to be saved together with the bucket index with the status of the last sync (bucket index update).
The bucket sync status is used on ingesters/queriers/storegateways/compactors to halt any operation for a given tenant if the bucket index could not be updated due CMK errors.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]