feat: prune invalid SSZ objects - #5388
Merged
Merged
Conversation
|
|
acuarica
marked this pull request as ready for review
April 19, 2023 15:20
Contributor
Performance Report鉁旓笍 no performance regression detected Full benchmark results
|
wemeetagain
previously requested changes
Apr 19, 2023
Contributor
Author
|
Hey @wemeetagain, thanks for your comments. I addressed all of them and left a question. |
dapplion
suggested changes
Apr 20, 2023
dapplion
left a comment
dapplion
left a comment
Contributor
There was a problem hiding this comment.
Good start! Main comment about moving code to CLI package
nflaig
reviewed
Apr 20, 2023
nflaig
reviewed
Apr 21, 2023
nflaig
reviewed
Apr 21, 2023
nflaig
reviewed
Apr 21, 2023
nflaig
left a comment
nflaig
left a comment
Member
There was a problem hiding this comment.
Some basic unit tests for this would be good to make sure files are deleted correctly and errors are handled
dapplion
force-pushed
the
acuarica/prune-invalid-ssz-objects
branch
from
May 29, 2023 12:54
189dd50 to
d17f200
Compare
Contributor
|
Redone, now with code in beacon handler only |
dapplion
reviewed
May 29, 2023
| // dev debug option to have access to the BN instance | ||
| if (args.attachToGlobalThis) { | ||
| (globalThis as unknown as {bn: BeaconNode}).bn = node; | ||
| } |
Contributor
There was a problem hiding this comment.
Added comment and this section of code is getting crowded
dapplion
force-pushed
the
acuarica/prune-invalid-ssz-objects
branch
from
May 29, 2023 12:57
d17f200 to
9d800ef
Compare
dapplion
previously approved these changes
May 29, 2023
nflaig
reviewed
May 29, 2023
nflaig
force-pushed
the
acuarica/prune-invalid-ssz-objects
branch
from
May 29, 2023 19:34
639c878 to
d1215f2
Compare
nflaig
previously approved these changes
May 29, 2023
| if (stat.isDirectory()) { | ||
| pruneOldFilesInDir(entryPath, maxAgeMs); | ||
| } else if (stat.isFile()) { | ||
| if (Date.now() - stat.mtimeMs > maxAgeMs) { |
Member
There was a problem hiding this comment.
Changed from cTimeMs to mtimeMs as can't manipulate creation time from within Nodejs which makes it hard to test.
I am not sure what is technically more correct. Currently, it should not matter for ssz objects because I would assume those are just written once and not modified.
For more general purpose, I would not consider a file which was just recently modified "old" so might be anyways better to use mTimeMs
nflaig
force-pushed
the
acuarica/prune-invalid-ssz-objects
branch
2 times, most recently
from
May 29, 2023 19:44
17c6fce to
9380aa6
Compare
nflaig
force-pushed
the
acuarica/prune-invalid-ssz-objects
branch
from
May 29, 2023 19:51
9380aa6 to
dd671c5
Compare
nflaig
approved these changes
May 29, 2023
dapplion
approved these changes
Jun 1, 2023
Member
|
馃帀 This PR is included in v1.9.0 馃帀 |
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
5 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.
Motivation
This PR closes #4420. That is, it allows the user to prune old
invalidSszObjectsafter a specified number of days.Description
This PR adds a new CLI hidden option
--persistInvalidSszObjectsRetentiononly applicable tobeaconcommand. It works in conjunction with--chain.persistInvalidSszObjectsand--persistInvalidSszObjectsDir.When
--chain.persistInvalidSszObjectsis enabled, pruning is activated by default. However, you can disable it with--persistInvalidSszObjectsRetention 0If
--persistInvalidSszObjectsRetentionis not specified, the retention period for invalid SSZ objects defaults to 7 days.The application prunes invalid SSZ objects during start up and every hour.
Steps to test or reproduce