Skip to content

[tests] make pruning.py faster #10591

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.

Sign up for GitHub

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

Jump to bottom
Closed
jnewbery wants to merge 2 commits into from
Closed

[tests] make pruning.py faster #10591

jnewbery wants to merge 2 commits into from

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Jun 14, 2017 •
edited
Loading

Headline: Brings time for pruning.py down to ~4 minutes on my VM when run without other tests in parallel.

A couple of modifications to make pruning.py much faster:

  • pruning.py doesn't care what kind of transactions make up the big blocks that are pruned in the test. Instead of making blocks with several large, expensive to construct and validate transactions, instead make the large blocks contain a single coinbase transaction with a huge OP_RETURN txout.
  • avoid stop-starting nodes where possible.

Builds on top of #10555 and #10556

My VM has slow disk i/o. This would possibly be faster with faster disk i/o. Unfortunately, pruning.py still isn't able to run on Travis. It appears that disk access is very slow for in the Travis environment, and this test still times out/fails. Unfortunately, it's not possible to run this test without very heavy disk access - the whole point of pruning is to remove block files when disk usage goes over a certain threshold.

I'd be interested to know how this affects runtime for others.

@sdaftuar

@fanquake fanquake added the Tests label Jun 15, 2017
@jnewbery jnewbery force-pushed the fastprune branch 2 times, most recently from 2463007 to 9e894d2 Compare June 16, 2017 16:40
@maflcko
Copy link
Member

maflcko commented Jun 18, 2017

Concept ACK. Needs rebase

@jnewbery
Copy link
Contributor Author

jnewbery commented Jun 18, 2017

I'll rebase if/when #10556 is merged, since I expect that may require further feedback and iteration. There's no rush for this PR since pruning.py isn't run by many people. Currently just looking for feedback on the concept and whether this helps reduce runtime for other people.

@maflcko
Copy link
Member

maflcko commented Jun 30, 2017

Since this runs a lot faster, can we also enable it for the travis cron job?

@jnewbery
Copy link
Contributor Author

jnewbery commented Jun 30, 2017

Since this runs a lot faster, can we also enable it for the travis cron job?

I don't think so. See the original description:

Unfortunately, pruning.py still isn't able to run on Travis. It appears that disk access is very slow for in the Travis environment, and this test still times out/fails. Unfortunately, it's not possible to run this test without very heavy disk access - the whole point of pruning is to remove block files when disk usage goes over a certain threshold.

If you can think of a cunning way to get this to run on Travis, that'd be great. I haven't been able to come up with anything.

@maflcko
Copy link
Member

maflcko commented Jul 2, 2017

Needs rebase

jnewbery added 2 commits July 2, 2017 22:05
@jnewbery
[tests] Introduce TestNode
91b0979 
TestNode is a class responsible for all state related to a bitcoind node
under test. It stores local state, is responsible for tracking the
bitcoind process and delegates unrecognised messages to the RPC
connection.

This commit changes start_nodes and stop_nodes to start and stop the
bitcoind nodes in parallel, making test setup and teardown much faster.
@jnewbery
[tests] make pruning test faster
806bf48 
This commit makes the pruning.py much faster.

Key insights to do this:

- pruning.py doesn't care what kind of transactions make up the big
blocks that are pruned in the test. Instead of making blocks with
several large, expensive to construct and validate transactions,
instead make the large blocks contain a single coinbase transaction with
a huge OP_RETURN txout.
- avoid stop-starting nodes where possible.

This test could probably be made even faster by using the P2P interface
for submitting blocks instead of the submitblock RPC.
@jnewbery
Copy link
Contributor Author

jnewbery commented Jul 2, 2017

rebased

@maflcko
Copy link
Member

maflcko commented Jul 3, 2017

Does this depend on the TestNode commit?

@fanquake
Copy link
Member

fanquake commented Jul 6, 2017

Testing this:

2017-07-06 01:08:57.788000 TestFramework (INFO): New best height: 1032
2017-07-06 01:08:57.788000 TestFramework (INFO): Generating new longer chain of 300 more blocks
2017-07-06 01:08:59.991000 TestFramework (INFO): Verify height on node 2: 1320
2017-07-06 01:08:59.992000 TestFramework (INFO): Usage possibly still high because of stale blocks in block files: 512
2017-07-06 01:08:59.992000 TestFramework (INFO): Mine 220 more large blocks so we have requisite history
2017-07-06 01:11:04.998000 TestFramework (INFO): Usage should be below target: 512
2017-07-06 01:11:04.999000 TestFramework (INFO): Test that we can rerequest a block we previously pruned if needed for a reorg
2017-07-06 01:11:06.476000 TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/Users/x/Github/bitcoin/test/functional/test_framework/test_framework.py", line 143, in main
    self.run_test()
  File "test/functional/pruning.py", line 436, in run_test
    self.reorg_back()
  File "test/functional/pruning.py", line 211, in reorg_back
    assert_raises_jsonrpc(-1, "Block not available (pruned data)", self.nodes[2].getblock, self.forkhash)
  File "/Users/x/Github/bitcoin/test/functional/test_framework/util.py", line 87, in assert_raises_jsonrpc
    raise AssertionError("No exception raised")
AssertionError: No exception raised
2017-07-06 01:11:06.477000 TestFramework (INFO): Stopping nodes

@maflcko
Copy link
Member

maflcko commented Aug 30, 2017

@jnewbery Needs rebase

@jnewbery
Copy link
Contributor Author

jnewbery commented Aug 31, 2017

closing this for now. It seems to work most of the time for me, but intermittently fails for the same reason that @fanquake gives above. There's refactoring work going on in the test_framework right now, so this is likely to require more rebases and it doesn't seem worth the effort given that very few people run this test.

@jnewbery jnewbery closed this Aug 31, 2017
@jtimon
Copy link
Contributor

jtimon commented Sep 1, 2017 •
edited
Loading

Fwiw, perhaps more people would run this test if it was faster. Personally I avoid it precisely because of how slow it is.

@jnewbery jnewbery mentioned this pull request Apr 11, 2018
Closed
This was referenced Mar 28, 2019
Merged
Merged
@adamjonas
Copy link
Member

adamjonas commented Oct 18, 2019

Can remove the up for grabs label with #15686 making this obsolete.

@jnewbery jnewbery deleted the fastprune branch October 18, 2019 20:28
@maflcko
Copy link
Member

maflcko commented Oct 18, 2019

Thanks @adamjonas

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jnewbery @maflcko @fanquake @jtimon @adamjonas