Perform final tenant cleanup after last block is deleted. #3613
Conversation
| RuleGroupsDeleted bool `json:"rule_groups_deleted,omitempty"` | ||
| AlertManagerConfigDeleted bool `json:"alert_manager_config_deleted,omitempty"` |
There was a problem hiding this comment.
The omitempty will cause rule_groups_deleted and alert_manager_config_deleted to be omitted from the response until their value is false. I think it's more clear if we always include them. Currently, they're not supported, but that's another way to clearly state rules and alertmanager config hasn't been deleted.
There was a problem hiding this comment.
Reason for hiding them was exactly because they are not supported yet. Once support is added, I would remove omitempty.
There was a problem hiding this comment.
Ok, as you prefer. I think it was clear to state they're not supported having them always exported with the false value, but your way works too.
pkg/compactor/blocks_cleaner.go
Outdated
| return nil | ||
| } | ||
|
|
||
| level.Info(userLogger).Log("msg", "cleaning up remaining tenant data") |
There was a problem hiding this comment.
Is this log message useful? If so, I would improve the log message. This log comes from the compactor and this generic message may not be clear to the user.
There was a problem hiding this comment.
I will improve it. I think it's useful to know when final deletion was done.
| } | ||
|
|
||
| // If we have just deleted some blocks, update "finished" time. Also update "finished" time if it wasn't set yet, but there are no blocks. | ||
| // Note: this UPDATES the tenant deletion mark. Components that use caching bucket will NOT SEE this update, |
There was a problem hiding this comment.
Any chance of multiple callers updating it? I assume if they do they will write similar times and it will be ok?
There was a problem hiding this comment.
Within one compactor, it is only called once.
Multiple compactors may happen to call this method concurrently (eg. one compactor is doing compaction cleanup, and another compactor starts and "takes over" the user). In that case, they may overwrite each-others timestamp if they both happen to delete some blocks or read tenant deletion mark with no timestamp. In that case last write will win, which will delay eventual final cleanup of data for tenant.
Thanks for your review. I've addressed your feedback, and PR is ready for another look. |
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
Signed-off-by: Peter Štibraný <peter.stibrany@grafana.com>
What this PR does: This PR implements final tenant cleanup after some delay since last tenant's block has been deleted. Cleanup will remove all remaining marker files (including tenant deletion), and meta.json files stored under
<tenant>/debug/metaprefix.Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]