-
Notifications
You must be signed in to change notification settings - Fork 1k
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 BlockBatchLimit comment: Maximum 128 #13502
Conversation
@@ -158,7 +158,7 @@ var ( | |||
// BlockBatchLimit specifies the requested block batch size. | |||
BlockBatchLimit = &cli.IntFlag{ | |||
Name: "block-batch-limit", | |||
Usage: "The amount of blocks the local peer is bounded to request and respond to in a batch.", | |||
Usage: "The amount of blocks the local peer is bounded to request and respond to in a batch. Maximum 128", |
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.
This number changes depending on network so just saying maximum 128 might not be great, we should think of a better way to word this
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.
got it. It seems that it's not so good to word this here because the range is dynamic.
What about adding param judgement on the fetchBlocksFromPeer to make user can realize it without enable the Debug otherwise user would keep waiting here because there is no any error msg . like this :
maxRequest := params.MaxRequestBlock(slots.ToEpoch(start))
if count > maxRequest {
log.WithField("count", count).WithField("maxRequest", maxRequest).Error("Requested count is too high, reducing")
}
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.
Let's merge this as-is for deneb and we can iterate / improve it later as needed.
What type of PR is this?
Documentation
What does this PR do? Why is it needed?
As mentioned in the issue, make people aware that there is a range limit of BlockBatchLimit. Especially since it was changed on the Dencun.
Which issues(s) does this PR fix?
Fixes #13499
Other notes for review