Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Producer plugin #7791

Merged
merged 7 commits into from
Aug 26, 2019
Merged

Producer plugin #7791

merged 7 commits into from
Aug 26, 2019

Conversation

heifner
Copy link
Contributor

@heifner heifner commented Aug 22, 2019

Change Description

  • Producer plugin was waking up a block early so that it would for sure not miss its production slot. However, this causes it to work twice as hard for the first block production. Under heavy load, setting exact wake up time works much better.

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@@ -1750,9 +1746,13 @@ void producer_plugin_impl::schedule_production_loop() {
auto expect_time = chain.pending_block_time() - fc::microseconds(config::block_interval_us);
// ship this block off up to 1 block time earlier or immediately
if (fc::time_point::now() >= expect_time) {
_timer.expires_from_now( boost::posix_time::microseconds( 0 ));
fc_dlog(_log, "Scheduling Block Production on Exhausted Block #${num} immediately",
// produce block immediately
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I cannot find a direct reason this is an issue, there are a few implicit requirements that this may one day run-afoul of that I think we should attempt to make more explicit

  1. schedule_production_loop is almost always called from a destructor of an fc::scoped_exit which means it really shouldn't throw (maybe one day we should consider requiring noexcept qualifiers on lambdas for fc::scoped_exit). Right now, maybe_produce_block` appears to handle all of the exceptions and is clean but I'd be interested in how we make sure the implicit requirement that it not throw is made explicit.

  2. maybe_produce_block calls schedule_production_loop making this recursive. I don't think it can get into an infinite recursion because this branch is taken based on time but, there are a lot of variables and if this stays direct, perhaps we should "schedule" the call to schedule_production_loop in maybe_produce_block so that it at least clears the stack ?

at the least we should probably make the log messages the same (other than the notice about exhaustion) and turn the call and the fc_dlogcalls into a local lambda so that the messages are kept consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted this change as I think it is clearer and more consistent the way it was.

@heifner heifner requested a review from b1bart August 26, 2019 14:17
@heifner heifner merged commit 591f9c6 into develop Aug 26, 2019
@heifner heifner deleted the opt-producer-plugin branch August 26, 2019 18:10
heifner added a commit that referenced this pull request Feb 4, 2020
@heifner heifner mentioned this pull request Feb 4, 2020
3 tasks
heifner added a commit that referenced this pull request Feb 4, 2020
@heifner heifner mentioned this pull request Feb 4, 2020
3 tasks
xiweihuang pushed a commit to BlockABC/eos that referenced this pull request Feb 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants