Added more use of ProgressLogger (#8504) #8509
Conversation
Signed-off-by: Vivkzz <[email protected]>
|
Did you run it? |
|
As in, run full pruning. |
|
|
||
| public PruningContext(FullPruningDb db, IDb cloningDb, bool duplicateReads) | ||
| { | ||
| CloningDb = cloningDb; | ||
| DuplicateReads = duplicateReads; | ||
| _db = db; | ||
| // Get total keys count in a more efficient way | ||
| TotalKeys = db._currentDb.GatherMetric().TotalKeys; |
There was a problem hiding this comment.
We will not be visiting all keys in database, so your upper bound will be off.
…8504) Signed-off-by: Vivkzz <[email protected]>
…github.com/Vivkzz/nethermind into feature/8504-add-progresslogger-to-pruning
|
@LukaszRozmej @asdacap Thanks for the feedback! I've made the following changes:
Regarding the question about running full pruning - I haven't run it yet. |
Forgot to push? |
|
@LukaszRozmej i already pushed it are u able to see it ? |
| if (!_committed) | ||
| { | ||
| // if the context was not committed, then pruning failed and we delete the cloned DB | ||
| CloningDb.Clear(); | ||
| } |
There was a problem hiding this comment.
- Fixed the concurrent batches issue by implementing
Interlocked.Incrementfor atomic counting of processed keys - Restored the code for clearing the cloned database on failure by:
- Reintroducing the
_committedflag - Adding back the cleanup code for failed pruning
- Adding proper disposal checks
- Reintroducing the
- Made the batch processing threshold consistent by setting it to 100,000 to match the individual processing threshold
All changes have been tested and include proper DCO sign-off. Please review the updates.
|
@LukaszRozmej kindly check and update me |
| public void Set(ReadOnlySpan<byte> key, byte[]? value, WriteFlags flags = WriteFlags.None) | ||
| { | ||
| _writeBatch.Set(key, value, flags); | ||
| _batchProcessedKeys++; |
There was a problem hiding this comment.
Oh wait. Probably fine here.
|
Can share some log? |
Fixes Closes Resolves #8504
Changes:
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
tested this locally, and the ProgressLogger works great, showing progress during the pruning process.
Documentation
Requires documentation update
If yes, link the PR to the docs update or the issue with the details labeled
docs. Remove if not applicable.Requires explanation in Release Notes
If yes, fill in the details here. Remove if not applicable.
Remarks
This change makes it easier to see what's happening during the pruning process, which is super helpful for long-running tasks.