[3.2] Enhancement: Add capability of not genereating block log by setting option block-log-retain-blocks to 0 #643
Conversation
| if (my->not_generate_block_log) { | ||
| // No blocks exist. Avoid cascading failures if going further. | ||
| return b; | ||
| } |
There was a problem hiding this comment.
There are a number of public methods of block_log that should also check this flag. Including: read_block, read_block_header, read_block_by_num, read_block_id_by_num, read_block_by_id, get_block_pos, read_head, flush, etc. These should all be updated to do the correct thing when in not_generate_block_log mode.
There was a problem hiding this comment.
Ideally we should have unittests around this new functionality so we can show all these work in the new mode.
|
|
||
| if ( my->chain_config->prune_config->prune_blocks == 0 ) { | ||
| // clear out empty blocks.log. otherwise block_log::extract_genesis_state | ||
| // will return version 0 which asserts. |
There was a problem hiding this comment.
How do you get into a state with a blocks.log of 0 size?
There was a problem hiding this comment.
I intentionally tested with blocks.log of 0 size. Probably it is not a valid test. Will revert this code.
| return False | ||
|
|
||
| def start_and_stop_nodeos(retain_blocks=0, produced_block=10): | ||
| cmd=f'programs/nodeos/nodeos -e -p eosio --plugin eosio::producer_api_plugin --plugin eosio::producer_plugin --plugin eosio::chain_api_plugin --plugin eosio::chain_plugin --data-dir {data_dir} --block-log-retain-blocks {retain_blocks}' |
There was a problem hiding this comment.
Should specify a --config-dir.
Also would be better to use Node.py or Cluster.py for launching.
There was a problem hiding this comment.
Also consider disabling p2p & http endpoints so it can be a parallelizable test. Can Node/Cluster do that for us @heifner ?
There was a problem hiding this comment.
You can start a cluster of 1 with extraNodeosArgs set with the --p2p-listen-endpoint. See almost any of the python tests.
One of the items I hope we can start on soon is making all the python integration tests able to run in parallel. By using Cluster.py when that work is done this test would pick put that ability along with it.
There was a problem hiding this comment.
I have rewritten the test using standard Cluster.py. I tried disabling p2p and http endpoints by setting --p2p-listen-endpoint and --http-server-address empty, but nodeos failed to come up due to the way Cluster.py starts nodeos. For now, I keep the test as nonparallel. Once the work of making python integration tests run in parallel is done, this test will pick up that ability without changes.
libraries/chain/block_log.cpp
Outdated
| prune_config.reset(); | ||
| not_generate_block_log = true; | ||
| } else { | ||
| EOS_ASSERT(prune_config->prune_blocks, block_log_exception, "block log prune configuration requires at least one block"); |
There was a problem hiding this comment.
this assert can be removed now
There was a problem hiding this comment.
Thanks. I missed that assert.
…take appropriate actions in block_log public functions when no block log mode is enabled and add corresponding unit tests, use update_head for all occasions
| TestHelper.printSystemInfo("BEGIN") | ||
| cluster.setWalletMgr(walletMgr) | ||
|
|
||
| cluster.setWalletMgr(walletMgr) |
There was a problem hiding this comment.
Thanks for careful reviewing! I am fixing it.
Resolve #92.
block-log-retain-blocksoption is modified to allow being set to 0, which tellsnodeosnot to generate blocks.log.Now
block-log-retain-blocksdescription looks likeNew unit tests and basic integration tests are added for this enhancement.