-
Notifications
You must be signed in to change notification settings - Fork 380
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
Async backing client ready #1302
Conversation
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.
In general looks good to me!
Just a few minor comments/questions.
Minimum allowed line rate is |
// We got around 500ms for proposing | ||
authoring_duration: Duration::from_millis(500), | ||
collation_request_receiver: Some(request_stream), | ||
authoring_duration: Duration::from_millis(1500), |
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.
Thinking about it now - shouldn't this still remain at 500?
At least until we turn on the async backing feature for some runtime?
E.g. we deploy this new client on the existing network now.
All collators still have half a second to propose the block, don't they?
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.
The block limit is defined on runtime by weight limit. This duration here is the client time available to build the block. If not built within 1.5 secs then it will timeout (hence slot hardware). Normally it needs to match with runtime block limit but this is a transitory period until async backing is enabled. We need this out so when the time comes, all clients support async backing and we just enable it.
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.
Right, weight limit
would be the limiter of the block size, assuming that the collator hardware is at least powerful as the benchmark machine.
Setting this value to 1500
now means that collators allow themselves to take up to 1500 [ms]
seconds to produce a block worth of 500 [ms]
(hence the addition of HW spec check as you said).
I was about to suggest to make this into a CLI param, but I assume that would defeat the purpose of avoiding the client restart 🙂.
I think it's all clear now. 👍
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.
Client is still required to build block within 0.5s, if not it will miss timeframe of 2s build&gossip. This doesn't mean they've 1.5s to build the block. It's more a time so client don't continue building if this timeout is reached. After this timeout, it's pointless to continue.
Update client to support async backing and apply necessary changes to runtime without enabling async-backing