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.
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
Add pruner for finalized states #8379
Add pruner for finalized states #8379
Changes from 14 commits
58736e4
82ff501
9c11ce1
afba944
87003ac
4cb4383
c8cefcb
82fd157
817165d
1560063
bc7af75
2b12580
3e447d6
6c6e3dd
191147d
ed73aa0
a01aa0a
0db66e7
e32612f
c238b1f
839320e
28869f5
cc904c3
88cbce0
a4821e2
0879674
f6e3052
1d84229
d290255
3b9611d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
we probably want to be mindful of logs here... maybe either put this to debug, or only log this when we're in archive mode...
also probalby want to think about this service WRT tree storage mode..
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.
yes, these are some leftovers I missed from a test. I'm doing a cleanup on some of those atm. the one that survive the clean up in the end they will all be at debug level
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.
changed this to be a more informative log but kept it at INFO level since it is only printed at startup, WDYT?
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.
i think if this runs during epoch transition it may get interesting, more of a general observation about all of these types of tasks i guess... we probably want to build out a design at some point that gives us more confidence about what parts of an epoch these tasks might be allowed...
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.
tbh, this has been the most difficult part so far, to find/define a good set up, I thought about something a little more dynamic. E.g having the interval being defined by epochs_retained and epoch_durantion
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.
I agree with Paul that we could have a "no flight time window" which could be epoch transition +/- 1 or 2 slots (ie from 31 to 2). The question will be how to handle tasks falling in that window:
Just dumped some thoughts.
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.
Another option is to link these prunings explicitly to certain slots, so the frequency will be an epoch for all of them, and we will have an implicit control over the epoch transition overlap (not guaranteed but reasonable).