-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[Optimize] Skip the scheduled batch proposal if already proposing #3112
[Optimize] Skip the scheduled batch proposal if already proposing #3112
Conversation
…gered one Signed-off-by: ljedrz <ljedrz@gmail.com>
// A best-effort attempt to skip the scheduled batch proposal if | ||
// round progression already triggered one. | ||
if self_.propose_lock.try_lock().is_err() { | ||
continue; |
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.
Perhaps add some debug!
message here for observing that there's an ongoing batch proposal attempt?
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 could be useful, but if my intuition is correct, such an occurrence could just be ignored (so a log would only increase verbosity); either way works for me.
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.
Resolved in 44f498b
The I believe this change is safe, but don't see that much of a difference in terms of resource optimizations. Let me know if I'm missing anything. |
@raychu86 you're right, it's nothing extreme, but it can avoid a duplicate proposal to be produced right after we produced one, which reduces CPU use and network traffic. |
Signed-off-by: Howard Wu <9260812+howardwu@users.noreply.github.com>
If I'm not missing anything, a batch proposal can be triggered by a timed loop or round progression, and if we're already proposing a batch for a new round, it seems reasonable to me that we may skip the timed one. This would save some network traffic and CPU use for the validators.
I tested it locally and can see the skip happening from time to time, which means those attempts can currently overlap.