HDDS-13365. OM support automatic table cache tracking and cleanup#10171
Open
ivandika3 wants to merge 6 commits intoapache:masterfrom
Open
HDDS-13365. OM support automatic table cache tracking and cleanup#10171ivandika3 wants to merge 6 commits intoapache:masterfrom
ivandika3 wants to merge 6 commits intoapache:masterfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
We need to ensure that the tables in CleanupTableInfo in OM response should be exactly the same as the ones updated in the corresponding OM request's validateAndUpdateCache. Previously, this is done with using
CleanupTableInfoannotations on theOMClientResponse. However, this is hard to keep track because all the cache updates are done in theOMClientRequest, but we need to update theOMClientResponseinstead, which is very error-prone. We should couple the table cache update inOMClientRequestand the tables to cleanup. That way, we remove theCleanupTableInforeflection entirely.One way is to track table cache to cleanup for the
OmClientResponsewhenever a cache is updated (e.g. using some context object). This will ensure that any table cache updated will eventually be cleaned up. Moreover, this can save some cleanup overhead if not all tables in theCleanupTableInfoneed to be cleaned up in certain cases (e.g. OMOpenKeysDeleteRequest might only need to cleanup only OPEN_KEY_TABLE (if all the open keys are from OBS / LEGACY buckets) or OPEN_FILE_TABLE (if all the open keys are from FSO bucket) or both. For example, for each Table#addCacheEntry, we also need to update the list of tables to cleanup. However, there might be some memory overhead when passing the OM table cache to cleanup (although the context objects should be short-lived).With this patch, we do not need to add any explicit
CleanupTableInfosince all cache entry is eligible to be cleaned up as soon as it's added. This would make OM request/response development less error-prone and contributors do not need to worry about cache cleanup mechanism while developing OM request and response.Note: Currently, the implementation uses ThreadLocal since currently OM validateAndUpdateCache only runs in a single thread (i.e. Ratis StateMachineUpdater) and the code changes is minimal. An alternative (less error-prone) idea is to pass tracker inside the
ExecutionContextwhich should be safer, but will require changes in a lot more places.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13365
How was this patch tested?
UT (Clean CI: https://github.com/ivandika3/ozone/actions/runs/25242237805)