rpc: Expose g_is_mempool_loaded via getmempoolinfo #15323

pull Empact wants to merge 2 commits into bitcoin:master from Empact:load-mempool-race changing 8 files +49 −27
  1. Empact commented at 10:14 PM on February 1, 2019: member

    And use it to fix a race condition in mempool_persist.py: https://travis-ci.org/Empact/bitcoin/jobs/487577243

    Since e.g. getrawmempool returns errors based on this status, this enables users to test it for readiness.

    Fixes #12863

  2. Empact force-pushed on Feb 1, 2019
  3. Empact force-pushed on Feb 1, 2019
  4. Empact force-pushed on Feb 1, 2019
  5. Empact commented at 10:46 PM on February 1, 2019: member

    The test failure, for reference:

    stdout:
2019-02-01T20:25:18.578000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20190201_201908/mempool_persist_74
2019-02-01T20:25:27.680000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/util.py", line 113, in try_rpc
    fun(*args, **kwds)
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/coverage.py", line 47, in __call__
    return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/authproxy.py", line 136, in __call__
    raise JSONRPCException(response['error'])
test_framework.authproxy.JSONRPCException: The mempool was not loaded yet (-1)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/test_framework.py", line 174, in main
    self.run_test()
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/mempool_persist.py", line 130, in run_test
    assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool)
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/util.py", line 105, in assert_raises_rpc_error
    assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
  File "/home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/test_framework/util.py", line 119, in try_rpc
    raise AssertionError("Expected substring not found:" + e.error['message'])
AssertionError: Expected substring not found:The mempool was not loaded yet
2019-02-01T20:25:27.682000Z TestFramework (INFO): Stopping nodes
2019-02-01T20:25:27.985000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20190201_201908/mempool_persist_74
2019-02-01T20:25:27.985000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20190201_201908/mempool_persist_74/test_framework.log
2019-02-01T20:25:27.985000Z TestFramework (ERROR): Hint: Call /home/travis/build/Empact/bitcoin/build/bitcoin-x86_64-unknown-linux-gnu/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20190201_201908/mempool_persist_74' to consolidate all logs
stderr:
  6. in src/rpc/blockchain.cpp:1512 in 869e66948e outdated 
    1508 | @@ -1508,6 +1509,7 @@ static UniValue getmempoolinfo(const JSONRPCRequest& request)
1509 |                  {},
1510 |                  RPCResult{
1511 |              "{\n"
1512 | +            "  \"loaded\": true|false         (boolean) True if the mempool has been loaded/initialized.\n"
    MarcoFalke commented at 11:04 PM on February 1, 2019:

    Should clarify that this means loaded from disk and if -persistmempool is on?

    Empact commented at 12:11 AM on February 2, 2019:

    If -persistmempool is off, g_is_mempool_loaded is set at the end of ThreadImport, after block import. Ideas on how to articulate that? Perhaps we could set it earlier in that case -persistmempool?

    promag commented at 1:14 AM on February 2, 2019:

    nit, why /initialized?

    Empact commented at 11:05 PM on February 2, 2019:

    Because it will also be true in cases where it has not been loaded from disk, but only "initialized". Maybe not worth mentioning.

    laanwj commented at 2:45 PM on February 12, 2019:

    Would prefer something like True if the mempool has been loaded from disk (if persistent)

    jnewbery commented at 4:55 PM on February 26, 2019:

    I think True if the mempool has been loaded from disk (if persistent), or need not be is much less clear than the more simple True if the mempool is fully loaded. The first raises more questions than it answers ("What is persistent? When need it not be?")

    MarcoFalke commented at 5:51 PM on February 26, 2019:

    Maybe s/persist/-persistmempool/g?

  7. MarcoFalke commented at 11:15 PM on February 1, 2019: member

    Ref #12863 (comment)

  8. jonasschnelli added the label RPC/REST/ZMQ on Feb 2, 2019
  9. Empact force-pushed on Feb 2, 2019
  10. promag commented at 1:14 AM on February 2, 2019: member

    Concept ACK. Could add a release note with the new key. This introduces a weird case, which is loaded can change from true to false. Maybe https://github.com/bitcoin/bitcoin/blob/b3a715301a0fd972fb2f3bd36e2680b3cdbbab26/src/init.cpp#L707 should change to

     g_is_mempool_loaded |= !ShutdownRequested();
  11. Empact force-pushed on Feb 2, 2019
  12. Empact force-pushed on Feb 2, 2019
  13. MarcoFalke added this to the milestone 0.19.0 on Feb 22, 2019
  14. DrahtBot commented at 4:24 PM on February 25, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  15. promag commented at 4:33 PM on February 25, 2019: member

    /rest/mempool/info.json is also affected.

  16. Empact force-pushed on Feb 26, 2019
  17. Empact force-pushed on Feb 26, 2019
  18. Empact force-pushed on Feb 26, 2019
  19. Empact force-pushed on Feb 26, 2019
  20. Empact commented at 9:40 AM on February 26, 2019: member

    Revised and cleaned up:

  21. Empact force-pushed on Mar 4, 2019
  22. Empact commented at 12:09 AM on March 4, 2019: member

    Updated comments and documentation as per John and Marco's suggestions #15323 (review)

  23. Empact force-pushed on Mar 4, 2019
  24. DrahtBot added the label Needs rebase on Mar 6, 2019
  25. Empact force-pushed on Mar 7, 2019
  26. Empact commented at 3:15 PM on March 7, 2019: member

    Rebase for #15473, and add another commit moving g_is_mempool_loaded into CTxMemPool, given that it is mempool-specific.

  27. DrahtBot removed the label Needs rebase on Mar 7, 2019
  28. in test/functional/mempool_persist.py:106 in 32e2018724 outdated 
     100 | @@ -100,14 +101,16 @@ def run_test(self):
 101 |          self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
 102 |          self.stop_nodes()
 103 |          self.start_node(0, extra_args=["-persistmempool=0"])
 104 | +        assert self.nodes[0].getmempoolinfo()["loaded"]
 105 |          # Give bitcoind a second to reload the mempool
 106 |          time.sleep(1)
    MarcoFalke commented at 4:45 PM on March 7, 2019:

    Can remove the sleep, given the assert?

  29. Empact force-pushed on Mar 9, 2019
  30. jnewbery commented at 5:06 PM on March 21, 2019: member

    ACK the first and third commits (rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json and Move g_is_mempool_loaded into CTxMemPool::m_is_loaded). I like the tidy up in Move g_is_mempool_loaded into CTxMemPool::m_is_loaded to remove the g_is_mempool_loaded global.

    I don't see the need for the second commit (rpc: Set g_is_mempool_loaded early if the mempool is not persisted). Having two places where m_is_loaded_ gets set seems to add complexity for no good reason. I don't think any users will see any behaviour difference before and after that commit.

  31. Empact force-pushed on Mar 21, 2019
  32. rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json
    And use it to fix a race condition in mempool_persist.py:
https://travis-ci.org/Empact/bitcoin/jobs/487577243

Since e.g. getrawmempool returns errors based on this status, this
enables users to test it for readiness.
    bb8ae2c419
  33. Move g_is_mempool_loaded into CTxMemPool::m_is_loaded
    So the loaded state is explicitly mempool-specific.
    effe81f750
  34. Empact force-pushed on Mar 22, 2019
  35. MarcoFalke commented at 10:44 PM on April 30, 2019: member

    utACK effe81f750

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

utACK effe81f750
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgQDgv/Vu+DeQYZkt0Au46dAPXOWF5uIfLHOGhOHbKB+YZ6zFNIrmw6VF5XrYDl
IJqLlqX2WtDQd8gYcQ9dDvVSBktaJrk6Qm9laEX8qITMavz5VHK8rHfqmJ9nrqC8
DNgWMlwhoQsJqflqXeaUKnkXv9eoMLvuHVn+F3kDkr9Wu/AjRjLxu1zaE9qnr0sl
lO8Z+mdVig4NDWbCHwZHyvP+eM8skn5WeEXTSi6Vl1yqgEP9T/9TEFQtlQ1ikTjn
t+ACMSxzfest155gsZt0vxn5ABlfw1phw3b/x6DEFIFNzs0cBYVRxu8kPhTarqYL
gMcrAvVlfs9knGDUaU4XaZbfor/cQRAduFCKqowSxob4jwb2FcWX9jDCxuyCBge7
kPS1qFNI0eBRw0OLSKKq3A5wH8Oyvx5XzagoRkFcKCz2hF1KqFfLKT+EyBViBWbR
lW6qMTO+A26VJsslpzaTpfnQrTU3mfyUxJB3+IPVQ0Mcibt2jBdfYJCq0/W+DIGe
RtTjJIFt
=u6Rr
-----END PGP SIGNATURE-----

    Timestamp of file with hash fcca790e597327e6dbf958e78e5e5c02c0911ef653160c591bf56423b214127d -

    </details>

  36. jnewbery commented at 11:50 PM on April 30, 2019: member

    utACK effe81f7503d2ca3c88cfdea687f9f997f353e0d @Empact - apologies for not reACKing this earlier. Force-pushes don't trigger new notifications in github. Feel free to ping me or add a comment to the PR if you need a reACK

  37. MarcoFalke merged this on May 1, 2019
  38. MarcoFalke closed this on May 1, 2019
  39. MarcoFalke referenced this in commit 12aa2ac988 on May 1, 2019
  40. sidhujag referenced this in commit 8a975dc539 on May 1, 2019
  41. Empact deleted the branch on May 2, 2019
  42. Mengerian referenced this in commit f316cd272e on Oct 31, 2019
  43. jonspock referenced this in commit 9107c980e9 on Dec 28, 2019
  44. jonspock referenced this in commit 42d6068686 on Dec 29, 2019
  45. jonspock referenced this in commit 7d544c6ecd on Dec 29, 2019
  46. furszy referenced this in commit a51b254f27 on Mar 20, 2021
  47. vijaydasmp referenced this in commit 185d1f3546 on Oct 21, 2021
  48. vijaydasmp referenced this in commit e27c1a393b on Oct 21, 2021
  49. vijaydasmp referenced this in commit 47375dbecd on Oct 22, 2021
  50. vijaydasmp referenced this in commit 8e8588a542 on Oct 22, 2021
  51. vijaydasmp referenced this in commit bfe027d095 on Oct 23, 2021
  52. vijaydasmp referenced this in commit b9561673fa on Oct 26, 2021
  53. vijaydasmp referenced this in commit 19ba92ca28 on Dec 6, 2021
  54. vijaydasmp referenced this in commit 1634550d9e on Dec 10, 2021
  55. vijaydasmp referenced this in commit d5e24debd4 on Dec 10, 2021
  56. vijaydasmp referenced this in commit 71941b7c23 on Dec 10, 2021
  57. vijaydasmp referenced this in commit 1520ebfaa2 on Dec 11, 2021
  58. vijaydasmp referenced this in commit f6bc96c0ca on Dec 11, 2021
  59. vijaydasmp referenced this in commit 9966abbd90 on Dec 11, 2021
  60. vijaydasmp referenced this in commit 92032654d6 on Dec 11, 2021
  61. vijaydasmp referenced this in commit 4a2dceb202 on Dec 11, 2021
  62. vijaydasmp referenced this in commit 75ad56190f on Dec 13, 2021
  63. vijaydasmp referenced this in commit c5f428c2d2 on Dec 13, 2021
  64. vijaydasmp referenced this in commit 3c44a4aa24 on Dec 13, 2021
  65. vijaydasmp referenced this in commit 4bb33614e8 on Dec 14, 2021
  66. vijaydasmp referenced this in commit b25e8edf15 on Dec 14, 2021
  67. vijaydasmp referenced this in commit 516865bf80 on Dec 15, 2021
  68. vijaydasmp referenced this in commit 497b047cc6 on Dec 15, 2021
  69. DrahtBot locked this on Dec 16, 2021
Contributors
EmpactMarcoFalkepromaglaanwjjnewberyDrahtBot
Labels
RPC/REST/ZMQ

Milestone
0.19.0

Linked (view graph)
#12863 mempool_persist.py failing in travis
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-22 06:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me