From c69dd986eff99b58ec5ce3e50a0f5f2c26cd1f6a Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 10:23:21 -0600 Subject: [PATCH 1/8] Fixed error statement to correctly indicate failure information. --- tests/nodeos_forked_chain_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodeos_forked_chain_test.py b/tests/nodeos_forked_chain_test.py index e17425c3257..59f23899fab 100755 --- a/tests/nodeos_forked_chain_test.py +++ b/tests/nodeos_forked_chain_test.py @@ -290,7 +290,7 @@ def getMinHeadAndLib(prodNodes): if producerToSlot[lastBlockProducer]["count"]!=inRowCountPerProducer: Utils.errorExit("Producer %s, in slot %d, expected to produce %d blocks but produced %d blocks. At block number %d." % - (blockProducer, slot, inRowCountPerProducer, producerToSlot[lastBlockProducer]["count"], blockNum)) + (lastBlockProducer, slot, inRowCountPerProducer, producerToSlot[lastBlockProducer]["count"], blockNum-1)) if blockProducer==productionCycle[0]: break From ab3a2aeb6e3cb500724106e10731b2fdda15d7df Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 12:40:27 -0600 Subject: [PATCH 2/8] Fixed name to be more descriptive. --- ...eos_multiple_version_protocol_feature_test.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/nodeos_multiple_version_protocol_feature_test.py b/tests/nodeos_multiple_version_protocol_feature_test.py index 02c4bf11ea9..7c4598ab350 100755 --- a/tests/nodeos_multiple_version_protocol_feature_test.py +++ b/tests/nodeos_multiple_version_protocol_feature_test.py @@ -117,7 +117,7 @@ def resumeBlockProductions(): for node in allNodes: if not node.killed: node.processCurlCmd("producer", "resume", "") - def shouldNodesBeInSync(nodes:[Node]): + def areNodesInSync(nodes:[Node]): # Pause all block production to ensure the head is not moving pauseBlockProductions() time.sleep(1) # Wait for some time to ensure all blocks are propagated @@ -129,7 +129,7 @@ def shouldNodesBeInSync(nodes:[Node]): return len(set(headBlockIds)) == 1 # Before everything starts, all nodes (new version and old version) should be in sync - assert shouldNodesBeInSync(allNodes), "Nodes are not in sync before preactivation" + assert areNodesInSync(allNodes), "Nodes are not in sync before preactivation" # First, we are going to test the case where: # - 1st node has valid earliest_allowed_activation_time @@ -147,13 +147,13 @@ def shouldNodesBeInSync(nodes:[Node]): assert shouldNodeContainPreactivateFeature(newNodes[0]), "1st node should contain PREACTIVATE FEATURE" assert not (shouldNodeContainPreactivateFeature(newNodes[1]) or shouldNodeContainPreactivateFeature(newNodes[2])), \ "2nd and 3rd node should not contain PREACTIVATE FEATURE" - assert shouldNodesBeInSync([newNodes[1], newNodes[2], oldNode]), "2nd, 3rd and 4th node should be in sync" - assert not shouldNodesBeInSync(allNodes), "1st node should be out of sync with the rest nodes" + assert areNodesInSync([newNodes[1], newNodes[2], oldNode]), "2nd, 3rd and 4th node should be in sync" + assert not areNodesInSync(allNodes), "1st node should be out of sync with the rest nodes" waitForOneRound() assert not shouldNodeContainPreactivateFeature(newNodes[0]), "PREACTIVATE_FEATURE should be dropped" - assert shouldNodesBeInSync(allNodes), "All nodes should be in sync" + assert areNodesInSync(allNodes), "All nodes should be in sync" # Then we set the earliest_allowed_activation_time of 2nd node and 3rd node with valid value # Once the 1st node activate PREACTIVATE_FEATURE, all of them should have PREACTIVATE_FEATURE activated in the next block @@ -167,8 +167,8 @@ def shouldNodesBeInSync(nodes:[Node]): libBeforePreactivation = newNodes[0].getIrreversibleBlockNum() newNodes[0].activatePreactivateFeature() - assert shouldNodesBeInSync(newNodes), "New nodes should be in sync" - assert not shouldNodesBeInSync(allNodes), "Nodes should not be in sync after preactivation" + assert areNodesInSync(newNodes), "New nodes should be in sync" + assert not areNodesInSync(allNodes), "Nodes should not be in sync after preactivation" for node in newNodes: assert shouldNodeContainPreactivateFeature(node), "New node should contain PREACTIVATE_FEATURE" activatedBlockNum = newNodes[0].getHeadBlockNum() # The PREACTIVATE_FEATURE should have been activated before or at this block num @@ -195,7 +195,7 @@ def shouldNodesBeInSync(nodes:[Node]): restartNode(oldNode, oldNodeId, chainArg="--replay", nodeosPath="programs/nodeos/nodeos") time.sleep(2) # Give some time to replay - assert shouldNodesBeInSync(allNodes), "All nodes should be in sync" + assert areNodesInSync(allNodes), "All nodes should be in sync" assert shouldNodeContainPreactivateFeature(oldNode), "4th node should contain PREACTIVATE_FEATURE" testSuccessful = True From 740a650ad99f5fe82e08afb45ccfe3af417db356 Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 12:41:32 -0600 Subject: [PATCH 3/8] Added extra sleep time to account for slower propogation of blocks through network. --- tests/nodeos_multiple_version_protocol_feature_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodeos_multiple_version_protocol_feature_test.py b/tests/nodeos_multiple_version_protocol_feature_test.py index 7c4598ab350..0101dd96909 100755 --- a/tests/nodeos_multiple_version_protocol_feature_test.py +++ b/tests/nodeos_multiple_version_protocol_feature_test.py @@ -120,7 +120,7 @@ def resumeBlockProductions(): def areNodesInSync(nodes:[Node]): # Pause all block production to ensure the head is not moving pauseBlockProductions() - time.sleep(1) # Wait for some time to ensure all blocks are propagated + time.sleep(2) # Wait for some time to ensure all blocks are propagated headBlockIds = [] for node in nodes: headBlockId = node.getInfo()["head_block_id"] From 52df6bedb0894751a264ec7a25f5a7fa0ee0bd27 Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 14:27:06 -0600 Subject: [PATCH 4/8] Added logging to indicate when production is paused and resumed. --- plugins/producer_plugin/producer_plugin.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 2e5d241787f..715bc345d16 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -963,6 +963,7 @@ void producer_plugin::handle_sighup() { } void producer_plugin::pause() { + ilog("Producer paused.") my->_pause_production = true; } @@ -974,7 +975,10 @@ void producer_plugin::resume() { if (my->_pending_block_mode == pending_block_mode::speculating) { chain::controller& chain = my->chain_plug->chain(); my->_unapplied_transactions.add_aborted( chain.abort_block() ); + ilog("Producer resumed. Scheduling production.") my->schedule_production_loop(); + } else { + ilog("Producer resumed.") } } From 115ed05b84b96fb1213ae93473b34a55fab2978d Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 14:27:54 -0600 Subject: [PATCH 5/8] Added block time offset for 1.7 node to avoid test errors due to stand your ground. --- tests/nodeos_multiple_version_protocol_feature_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/nodeos_multiple_version_protocol_feature_test.py b/tests/nodeos_multiple_version_protocol_feature_test.py index 0101dd96909..aed9db975e7 100755 --- a/tests/nodeos_multiple_version_protocol_feature_test.py +++ b/tests/nodeos_multiple_version_protocol_feature_test.py @@ -97,7 +97,8 @@ def hasBlockBecomeIrr(): specificExtraNodeosArgs={ 0:"--http-max-response-time-ms 990000", 1:"--http-max-response-time-ms 990000", - 2:"--http-max-response-time-ms 990000"}, + 2:"--http-max-response-time-ms 990000", + 3:"--last-block-time-offset-us -200000"}, onlySetProds=True, pfSetupPolicy=PFSetupPolicy.NONE, alternateVersionLabelsFile=alternateVersionLabelsFile, From 7e819c17e0d5523bc76634926b387650eeb1f54a Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Thu, 27 Feb 2020 17:09:59 -0600 Subject: [PATCH 6/8] Fixed code errors. --- plugins/producer_plugin/producer_plugin.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 715bc345d16..cae108148bf 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -963,7 +963,7 @@ void producer_plugin::handle_sighup() { } void producer_plugin::pause() { - ilog("Producer paused.") + ilog("Producer paused."); my->_pause_production = true; } @@ -975,10 +975,10 @@ void producer_plugin::resume() { if (my->_pending_block_mode == pending_block_mode::speculating) { chain::controller& chain = my->chain_plug->chain(); my->_unapplied_transactions.add_aborted( chain.abort_block() ); - ilog("Producer resumed. Scheduling production.") + ilog("Producer resumed. Scheduling production."); my->schedule_production_loop(); } else { - ilog("Producer resumed.") + ilog("Producer resumed."); } } From 83d5704189409bc762782d26654b1a4afe735f11 Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Fri, 28 Feb 2020 13:24:13 -0600 Subject: [PATCH 7/8] Cleaned up logs. --- plugins/producer_plugin/producer_plugin.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index cae108148bf..07e3ed9efc8 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -963,7 +963,7 @@ void producer_plugin::handle_sighup() { } void producer_plugin::pause() { - ilog("Producer paused."); + fc_ilog(_log, "Producer paused."); my->_pause_production = true; } @@ -975,10 +975,10 @@ void producer_plugin::resume() { if (my->_pending_block_mode == pending_block_mode::speculating) { chain::controller& chain = my->chain_plug->chain(); my->_unapplied_transactions.add_aborted( chain.abort_block() ); - ilog("Producer resumed. Scheduling production."); + fc_ilog(_log, "Producer resumed. Scheduling production."); my->schedule_production_loop(); } else { - ilog("Producer resumed."); + fc_ilog(_log, "Producer resumed."); } } From e5a54cd48fc2e6394adb438d7854aaa494806877 Mon Sep 17 00:00:00 2001 From: Brian Johnson Date: Fri, 28 Feb 2020 13:30:05 -0600 Subject: [PATCH 8/8] Added test comment to explain the needed parameter. --- tests/nodeos_multiple_version_protocol_feature_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/nodeos_multiple_version_protocol_feature_test.py b/tests/nodeos_multiple_version_protocol_feature_test.py index aed9db975e7..e45ffe45d59 100755 --- a/tests/nodeos_multiple_version_protocol_feature_test.py +++ b/tests/nodeos_multiple_version_protocol_feature_test.py @@ -91,6 +91,8 @@ def hasBlockBecomeIrr(): } Utils.Print("Alternate Version Labels File is {}".format(alternateVersionLabelsFile)) assert exists(alternateVersionLabelsFile), "Alternate version labels file does not exist" + # version 1.7 did not provide a default value for "--last-block-time-offset-us" so this is needed to + # avoid dropping late blocks assert cluster.launch(pnodes=4, totalNodes=4, prodCount=1, totalProducers=4, extraNodeosArgs=" --plugin eosio::producer_api_plugin ", useBiosBootFile=False,