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
chain: ShardManager: add ability to serve partial chunks from ShardChunk #6377
chain: ShardManager: add ability to serve partial chunks from ShardChunk #6377
Changes from 6 commits
22c0844
b1c7690
f6ea3da
516933b
2bdddba
a260126
c7d115c
1c7be71
c0231c0
34890c3
216628c
2a31ded
b6d18b1
87b2a58
f128a16
3f681fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
comment. Also maybe we should call it 'reconstruct_partial_chunk_on_the_fly' or something like that?
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’m planning to make GCing a configurable feature.
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.
maybe we should move it to a separate file (to keep this lib.rs a little bit smaller?)
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.
It’s not immediately obvious to be me what should be moved to separate file though to be honest. Code that handles partial encoded chunk requests is less than 250 lines so moving just it wouldn’t really help reduce the size of lib.rs which is 2700 lines. This would need some more care and thought to figure out which of the methods in the struct belong together in a separate file.
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.
Yeah, I don't think this makes sense to move to a separate file, as it seems to be pretty entangled with the rest.
The easiest wins in terms of keeping files small and logic clean are usually around various small helpers. Things like
RequestPool
orSealsManager
seem like better candidates for extraction.