added rate limiting to address biased trie cache preloading #2029
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (89.41%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2029 +/- ##
========================================
Coverage 49.57% 49.57%
========================================
Files 874 874
Lines 150499 150595 +96
========================================
+ Hits 74605 74653 +48
- Misses 70856 70901 +45
- Partials 5038 5041 +3
... and 20 files with indirect coverage changes
🚀 New features to boost your workflow:
|
|
pratikspatil024
changed the title
cffls
left a comment
cffls
left a comment
There was a problem hiding this comment.
Looks good overall! What long does it take to preload mainnet with the default throttling setting?
| // Apply rate limiting after reading, based on actual bytes read | ||
| if limiter != nil { | ||
| if err := limiter.WaitN(c.ctx, len(nodeData)); err != nil { | ||
| log.Info("Preload interrupted during rate limit wait", |
There was a problem hiding this comment.
Would this line flood the log?
| "max depth", maxDepthReached, | ||
| "size", common.StorageSize(totalBytesLoaded).String(), | ||
| "elapsed", time.Since(startTime)) | ||
| return |
There was a problem hiding this comment.
Not sure why the function returns here. If we return here it means the cache won't be warmed.
Description
Please provide a detailed description of what was done in this PR
Changes
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Nodes audience
In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)
Checklist
Cross repository changes
Testing
Manual tests
Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it
Additional comments
Please post additional comments in this section if you have them, otherwise delete it