From db98dbc5814accf4e9936d0c34ee65ef6e6832fc Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 11:52:39 -0500 Subject: [PATCH 1/8] GH-1690 Socket should only be accessed from thread-pool thread. Use async_write instead of send. Wait for all write callbacks to finish before exit. --- tests/trx_generator/trx_provider.cpp | 31 +++++++++++++++++++++++----- tests/trx_generator/trx_provider.hpp | 15 +++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index 190c6b4632..7ace099ac3 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -73,15 +73,36 @@ namespace eosio::testing { } void p2p_connection::disconnect() { - ilog("Closing socket."); - _p2p_socket.close(); - ilog("Socket closed."); + int max = 30; + int waited = 0; + while (_sent.load() != _sent_callback_num.load() && waited < max) { + ilog("disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", + ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + sleep(1); + ++waited; + } + if (waited == max) { + elog("disconnect failed to receive all acks in time - sent ${s} | acked ${a} | waited ${w}", + ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + } } void p2p_connection::send_transaction(const chain::packed_transaction& trx) { send_buffer_type msg = create_send_buffer(trx); - _p2p_socket.send(boost::asio::buffer(*msg)); - trx_acknowledged(trx.id(), fc::time_point::min()); //using min to identify ack time as not applicable for p2p + + ++_sent; + _strand.post( [this, msg{std::move(msg)}, id{trx.id()}]() { + boost::asio::async_write( _p2p_socket, boost::asio::buffer(*msg), + boost::asio::bind_executor( _strand, [this, msg, id]( boost::system::error_code ec, std::size_t w ) { + if (ec) { + elog("async write failure: ${e}", ("e", ec.message())); + } else { + trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p + } + ++_sent_callback_num; + }) + ); + } ); } acked_trx_trace_info p2p_connection::get_acked_trx_trace_info(const eosio::chain::transaction_id_type& trx_id) { diff --git a/tests/trx_generator/trx_provider.hpp b/tests/trx_generator/trx_provider.hpp index f11cb13f09..b53536ca74 100644 --- a/tests/trx_generator/trx_provider.hpp +++ b/tests/trx_generator/trx_provider.hpp @@ -2,9 +2,13 @@ #include #include -#include -#include #include + +#include + +#include +#include + #include #include #include @@ -104,10 +108,15 @@ namespace eosio::testing { struct p2p_connection : public provider_connection { boost::asio::ip::tcp::socket _p2p_socket; + boost::asio::io_context::strand _strand; + std::atomic _sent_callback_num{0}; + std::atomic _sent{0}; + explicit p2p_connection(const provider_base_config& provider_config) : provider_connection(provider_config) - , _p2p_socket(_connection_thread_pool.get_executor()) {} + , _p2p_socket(_connection_thread_pool.get_executor()) + , _strand(_connection_thread_pool.get_executor()){} void send_transaction(const chain::packed_transaction& trx) final; From 82e214634c45109c94b4fab813566d128c2ed88d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 11:54:00 -0500 Subject: [PATCH 2/8] GH-1690 Need to wait over time of run, so offset from startBlock. Make --print-missing-transactions=True the default as it is useful when there are missing trxs. --- tests/PerformanceHarness/log_reader.py | 2 +- tests/PerformanceHarness/performance_test_basic.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/PerformanceHarness/log_reader.py b/tests/PerformanceHarness/log_reader.py index ef68d4b4d5..388596c4b0 100644 --- a/tests/PerformanceHarness/log_reader.py +++ b/tests/PerformanceHarness/log_reader.py @@ -39,7 +39,7 @@ class TpsTestConfig: numTrxGensUsed: int = 0 targetTpsPerGenList: List[int] = field(default_factory=list) quiet: bool = False - printMissingTransactions: bool=False + printMissingTransactions: bool=True @dataclass class stats(): diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index 0d56329985..47d01bb242 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -171,7 +171,7 @@ class PtbConfig: quiet: bool=False delPerfLogs: bool=False expectedTransactionsSent: int = field(default_factory=int, init=False) - printMissingTransactions: bool=False + printMissingTransactions: bool=True userTrxDataFile: Path=None endpointMode: str="p2p" apiEndpoint: str=None @@ -495,7 +495,7 @@ def configureConnections(): scrapeTrxGenTrxSentDataLogs(trxSent, self.trxGenLogDirPath, self.ptbConfig.quiet) if len(trxSent) != self.ptbConfig.expectedTransactionsSent: print(f"ERROR: Transactions generated: {len(trxSent)} does not match the expected number of transactions: {self.ptbConfig.expectedTransactionsSent}") - blocksToWait = 2 * self.ptbConfig.testTrxGenDurationSec + 10 + blocksToWait = self.data.startBlock + (2 * self.ptbConfig.testTrxGenDurationSec) + 10 trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, blocksToWait) self.data.ceaseBlock = self.validationNode.getHeadBlockNum() @@ -733,7 +733,7 @@ def _createBaseArgumentParser(defEndpointApiDef: str, defProdNodeCnt: int, defVa ptbBaseParserGroup.add_argument("--save-state", help=argparse.SUPPRESS if suppressHelp else "Whether to save node state. (Warning: large disk usage)", action='store_true') ptbBaseParserGroup.add_argument("--quiet", help=argparse.SUPPRESS if suppressHelp else "Whether to quiet printing intermediate results and reports to stdout", action='store_true') ptbBaseParserGroup.add_argument("--prods-enable-trace-api", help=argparse.SUPPRESS if suppressHelp else "Determines whether producer nodes should have eosio::trace_api_plugin enabled", action='store_true') - ptbBaseParserGroup.add_argument("--print-missing-transactions", help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", action='store_true') + ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", default=True) ptbBaseParserGroup.add_argument("--account-name", type=str, help=argparse.SUPPRESS if suppressHelp else "Name of the account to create and assign a contract to", default="eosio") ptbBaseParserGroup.add_argument("--contract-dir", type=str, help=argparse.SUPPRESS if suppressHelp else "Path to contract dir", default="unittests/contracts/eosio.system") ptbBaseParserGroup.add_argument("--wasm-file", type=str, help=argparse.SUPPRESS if suppressHelp else "WASM file name for contract", default="eosio.system.wasm") From 339f5424659ee231c652de9e394d48a6bf5d659e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 13:28:21 -0500 Subject: [PATCH 3/8] GH-1690 Changed waitForTransactionsInBlockRange to use a start/end instead of start/offset. Pass in the start of empty blocks as the end of the range. --- tests/PerformanceHarness/performance_test_basic.py | 4 ++-- tests/TestHarness/Node.py | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index 47d01bb242..a6fc881057 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -491,12 +491,12 @@ def configureConnections(): completedRun = True # Get stats after transaction generation stops + endBlock = self.waitForEmptyBlocks(self.validationNode, self.emptyBlockGoal) trxSent = {} scrapeTrxGenTrxSentDataLogs(trxSent, self.trxGenLogDirPath, self.ptbConfig.quiet) if len(trxSent) != self.ptbConfig.expectedTransactionsSent: print(f"ERROR: Transactions generated: {len(trxSent)} does not match the expected number of transactions: {self.ptbConfig.expectedTransactionsSent}") - blocksToWait = self.data.startBlock + (2 * self.ptbConfig.testTrxGenDurationSec) + 10 - trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, blocksToWait) + trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, endBlock) self.data.ceaseBlock = self.validationNode.getHeadBlockNum() return PerformanceTestBasic.PtbTpsTestResult(completedRun=completedRun, numGeneratorsUsed=tpsTrxGensConfig.numGenerators, diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index b529990b6f..1a166de7e5 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -161,19 +161,18 @@ def checkBlockForTransactions(self, transIds, blockNum): transIds.pop(self.fetchTransactionFromTrace(trx)) return transIds - def waitForTransactionsInBlockRange(self, transIds, startBlock=2, maxFutureBlocks=0): + def waitForTransactionsInBlockRange(self, transIds, startBlock, endBlock): nextBlockToProcess = startBlock - overallEndBlock = startBlock + maxFutureBlocks while len(transIds) > 0: currentLoopEndBlock = self.getHeadBlockNum() - if currentLoopEndBlock > overallEndBlock: - currentLoopEndBlock = overallEndBlock + if currentLoopEndBlock > endBlock: + currentLoopEndBlock = endBlock for blockNum in range(nextBlockToProcess, currentLoopEndBlock + 1): transIds = self.checkBlockForTransactions(transIds, blockNum) if len(transIds) == 0: return transIds nextBlockToProcess = currentLoopEndBlock + 1 - if currentLoopEndBlock == overallEndBlock: + if currentLoopEndBlock == endBlock: Utils.Print("ERROR: Transactions were missing upon expiration of waitOnblockTransactions") break self.waitForHeadToAdvance() From bfe2a4c93918ac1665bffcd35e3ad6d8ec3fbdd7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 25 Oct 2023 12:19:34 -0500 Subject: [PATCH 4/8] GH-1690 Add backwards compatibility for `--produce-block-offset-ms 0` for performance harness --- tests/TestHarness/Cluster.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index fc0a6b055a..cc954ba36a 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -461,6 +461,11 @@ def connectGroup(group, producerNodes, bridgeNodes) : if "--plugin eosio::history_api_plugin" in args: argsArr.append("--is-nodeos-v2") break + + # Handle common case of specifying no block offset for older versions + if "v2" in self.nodeosVers or "v3" in self.nodeosVers or "v4" in self.nodeosVers: + argsArr = list(map(lambda st: str.replace(st, "--produce-block-offset-ms 0", "--last-block-time-offset-us 0 --last-block-cpu-effort-percent 100"), argsArr)) + Cluster.__LauncherCmdArr = argsArr.copy() launcher = cluster_generator(argsArr) From b79d98dd91bd1b8d03ffbc337234a67ee38ec814 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 27 Oct 2023 08:54:03 -0500 Subject: [PATCH 5/8] GH-1690 Not allowed to write to socket until previous write completes. --- tests/trx_generator/trx_provider.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index 7ace099ac3..eb946245f9 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -92,16 +92,9 @@ namespace eosio::testing { ++_sent; _strand.post( [this, msg{std::move(msg)}, id{trx.id()}]() { - boost::asio::async_write( _p2p_socket, boost::asio::buffer(*msg), - boost::asio::bind_executor( _strand, [this, msg, id]( boost::system::error_code ec, std::size_t w ) { - if (ec) { - elog("async write failure: ${e}", ("e", ec.message())); - } else { - trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p - } - ++_sent_callback_num; - }) - ); + boost::asio::write(_p2p_socket, boost::asio::buffer(*msg)); + trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p + ++_sent_callback_num; } ); } From bed5a26bac087e1b77a9b1988b5841e870e1a4a5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 Nov 2023 10:52:19 -0500 Subject: [PATCH 6/8] GH-1690 Report values used in comparison --- tests/trx_generator/trx_provider.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index eb946245f9..4ff29007fd 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -75,9 +75,11 @@ namespace eosio::testing { void p2p_connection::disconnect() { int max = 30; int waited = 0; - while (_sent.load() != _sent_callback_num.load() && waited < max) { + for (uint64_t sent = _sent.load(), sent_callback_num = _sent_callback_num.load(); + sent != sent_callback_num && waited < max; + sent = _sent.load(), sent_callback_num = _sent_callback_num.load()) { ilog("disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", - ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + ("s", sent)("a", sent_callback_num)("w", waited)); sleep(1); ++waited; } From db817f794e1a6dd9cfc3b4a1050c12cbd7517b18 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 10:29:02 -0500 Subject: [PATCH 7/8] GH-1690 Remove unneeded include --- tests/trx_generator/trx_provider.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/trx_generator/trx_provider.hpp b/tests/trx_generator/trx_provider.hpp index b53536ca74..f5685fe06b 100644 --- a/tests/trx_generator/trx_provider.hpp +++ b/tests/trx_generator/trx_provider.hpp @@ -4,8 +4,6 @@ #include #include -#include - #include #include From b0f567d4be75caed0e42f30a778f46285ac8d737 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 10:29:24 -0500 Subject: [PATCH 8/8] GH-1690 Add better description --- tests/PerformanceHarness/performance_test_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index a6fc881057..a96f24ea83 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -733,7 +733,7 @@ def _createBaseArgumentParser(defEndpointApiDef: str, defProdNodeCnt: int, defVa ptbBaseParserGroup.add_argument("--save-state", help=argparse.SUPPRESS if suppressHelp else "Whether to save node state. (Warning: large disk usage)", action='store_true') ptbBaseParserGroup.add_argument("--quiet", help=argparse.SUPPRESS if suppressHelp else "Whether to quiet printing intermediate results and reports to stdout", action='store_true') ptbBaseParserGroup.add_argument("--prods-enable-trace-api", help=argparse.SUPPRESS if suppressHelp else "Determines whether producer nodes should have eosio::trace_api_plugin enabled", action='store_true') - ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", default=True) + ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Print missing transactions upon test completion.", default=True) ptbBaseParserGroup.add_argument("--account-name", type=str, help=argparse.SUPPRESS if suppressHelp else "Name of the account to create and assign a contract to", default="eosio") ptbBaseParserGroup.add_argument("--contract-dir", type=str, help=argparse.SUPPRESS if suppressHelp else "Path to contract dir", default="unittests/contracts/eosio.system") ptbBaseParserGroup.add_argument("--wasm-file", type=str, help=argparse.SUPPRESS if suppressHelp else "WASM file name for contract", default="eosio.system.wasm")