Stopped tracking fate age off data in memory#4169
Merged
Merged
Conversation
cshannon
approved these changes
Jan 20, 2024
Contributor
cshannon
left a comment
There was a problem hiding this comment.
This LGTM, I like the new FateIdStatus and think that is a nice optimization to avoid reading twice.
Another nice thing is this PR also fixes #4166 and the error no longer shows up anymore in the in tests. This is fixed because the previous version with the Age off store immediately scanned in the constructor the transactions which caused the issue as described in #4166. This PR changes things so that the new FateCleaner task is scheduled with an initial delay of 10 minutes so it's a non-issue as by the time the cleaner runs for the first time the manager will be started and the RPC channel will be available.
The Accumulo store can very efficiently gather status the status at the same time as the id for a FATE transaction. This is already being gathered internally in implementation of the fate storage layer. This commit exposes this information further as it will be useful in refactoring the ageoff store.
Replaced Fate AgeOffStore with FateCleaner. FateCleaner tracks fate operations that are candidates for age off in the fate storage layer instead of in memory. Also FateCleaner does not wrap a fate storage layer, it just takes one to track. fixes apache#4130
2286cf3 to
be36cde
Compare
keith-turner
added a commit
that referenced
this pull request
Jan 20, 2024
The Accumulo store can very efficiently gather status the status at the same time as the id for a FATE transaction. This is already being gathered internally in implementation of the fate storage layer. This commit exposes this information further as it will be useful in refactoring the ageoff store.
cshannon
added a commit
to cshannon/accumulo
that referenced
this pull request
Jan 26, 2024
The changes in apache#4169 moved age off tracking out of memory and into the FATE store. The AccumuloStore has a bug where it doesn't handle the new TxInfo enum type of TX_AGEOFF so the info is never persisted to the store. This fixes the store so it will correctly read/write the value and also adds a default enum case to throw an exception in the future in case a new value is passed that is unknown so it will be caught. A test was also added to iterate over all types to verify they all work for the AccumuloStore so if a new info type is ever added it will validate it in the future.
cshannon
added a commit
that referenced
this pull request
Jan 26, 2024
The changes in #4169 moved age off tracking out of memory and into the FATE store. The AccumuloStore has a bug where it doesn't handle the new TxInfo enum type of TX_AGEOFF so the info is never persisted to the store. This fixes the store so it will correctly read/write the value and also adds a default enum case to throw an exception in the future in case a new value is passed that is unknown so it will be caught. A test was also added to iterate over all types to verify they all work for the AccumuloStore so if a new info type is ever added it will validate it in the future. Co-authored-by: Keith Turner <kturner@apache.org>
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
3 participants
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.
These changes stop tracking fate age off data in memory which will avoid using too much memory if there are lots of fate operations. There are two commits, the first commit contains prerequisite changes to the fate storage layer that were needed to move age off tracking into persistent storage. The second commit moves age off tracking from memory to persistent storage.