Control mempool persistence using a command line parameter #9966

pull jnewbery wants to merge 2 commits into bitcoin:master from jnewbery:mempoolpersistenceoption changing 4 files +101 −4
  1. jnewbery commented at 10:19 pm on March 9, 2017: member

    Mempool persistence was added in 3f78562df5e86a2a0a21812047fc3a7db8cee988, and is always on. This commit introduces a command-line parameter -persistmempool, which defaults to true. When set to false:

    • mempool.dat is not loaded when the node starts.
    • mempool.dat is not written when the node stops.

    This was suggested in the PR that introduced mempool persistence (https://github.com/bitcoin/bitcoin/pull/8448#issuecomment-237155942 and #8448 (comment)) , but wasn’t implemented in the original PR. It is a pre-req for fixing #9710.

    This PR also introduces tests for mempool persistence, as well as for the new command-line parameter.

  2. fanquake added the label Mempool on Mar 9, 2017
  3. luke-jr commented at 10:20 pm on March 9, 2017: member

    Concept ACK.

    Idea: Is there any reason a user might want to load or dump but not both?

    Maybe it should be a debug option… dunno.

  4. in src/init.cpp: in 89acdb0307 outdated 
    374@@ -374,6 +375,7 @@ std::string HelpMessage(HelpMessageMode mode)
375     strUsage += HelpMessageOpt("-maxtimeadjustment", strprintf(_("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)"), DEFAULT_MAX_TIME_ADJUSTMENT));
376     strUsage += HelpMessageOpt("-onion=<ip:port>", strprintf(_("Use separate SOCKS5 proxy to reach peers via Tor hidden services (default: %s)"), "-proxy"));
377     strUsage += HelpMessageOpt("-onlynet=<net>", _("Only connect to nodes in network <net> (ipv4, ipv6 or onion)"));
378+    strUsage += HelpMessageOpt("-persistmempool", strprintf(_("Whether to dump the mempool on shutdown and load on restart(default: %u)"), DEFAULT_PERSIST_MEMPOOL));
    laanwj commented at 6:58 am on March 10, 2017:
    I’d use “save” instead of “dump” especially if this is to be an official translated option and not a debug option. Also missing a space before (
  5. laanwj commented at 6:59 am on March 10, 2017: member

    utACK apart from the nit, seems straightforward enough.

    We should indeed consider the debug/non-debug status of this option. Is it worth adding translation strings for this, or is it just for technical debugging/development purposes? Not entirely decided about this.

  6. paveljanik commented at 7:03 am on March 10, 2017: contributor
    @luke-jr there is. I had the same idea while debugging #9810. I wanted to load the original mempool.dat, but not rewrite it at the exit. I solved it by copying mempool.dat back before the next start. And as the workaround is so simple, please let’s not complicate the code with yet another option.
  7. laanwj commented at 7:04 am on March 10, 2017: member

    And as the workaround is so simple, please let’s not complicate the code with yet another option.

    Agreed. So to be clear adding this option is good, but let’s not add micro-management for loading/saving specifically.

  8. in qa/rpc-tests/mempool_persist.py: in 89acdb0307 outdated 
    0@@ -0,0 +1,89 @@
1+#!/usr/bin/env python3
2+# Copyright (c) 2014-2016 The Bitcoin Core developers
    paveljanik commented at 7:05 am on March 10, 2017:
    17
  9. paveljanik approved
  10. paveljanik commented at 7:13 am on March 10, 2017: contributor

    tested ACK https://github.com/bitcoin/bitcoin/pull/9966/commits/89acdb0307bb00c41142753bcba6629136f672a0

    I especially like the test and its description!

  11. jnewbery force-pushed on Mar 10, 2017
  13. TheBlueMatt commented at 3:59 pm on March 10, 2017: member
    utACK c4e615429458f8c2159d290082940bef2fe4f263
  14. paveljanik commented at 10:02 pm on March 10, 2017: contributor
    ACK c4e6154
  15. in src/init.cpp: in c4e6154294 outdated 
    374@@ -374,6 +375,7 @@ std::string HelpMessage(HelpMessageMode mode)
375     strUsage += HelpMessageOpt("-maxtimeadjustment", strprintf(_("Maximum allowed median peer time offset adjustment. Local perspective of time may be influenced by peers forward or backward by this amount. (default: %u seconds)"), DEFAULT_MAX_TIME_ADJUSTMENT));
376     strUsage += HelpMessageOpt("-onion=<ip:port>", strprintf(_("Use separate SOCKS5 proxy to reach peers via Tor hidden services (default: %s)"), "-proxy"));
377     strUsage += HelpMessageOpt("-onlynet=<net>", _("Only connect to nodes in network <net> (ipv4, ipv6 or onion)"));
378+    strUsage += HelpMessageOpt("-persistmempool", strprintf(_("Whether to save the mempool on shutdown and load on restart (default: %u)"), DEFAULT_PERSIST_MEMPOOL));
    MarcoFalke commented at 10:24 pm on March 10, 2017:
    I might be missing something, but this is not a “connection option”. Also, this could be formatted as an untranslated debug option.
    paveljanik commented at 10:29 pm on March 10, 2017:

    Good catch, Marco! It should be moved out from “Connection options” help, e.g. after the mempoolexpiry help?

    I think it should be normal option, non-debug, ie. translated.

  16. sipa commented at 6:28 am on March 12, 2017: member
    utACK, but move the option in the help section as suggested.
  17. jnewbery force-pushed on Mar 15, 2017
  18. jnewbery commented at 3:44 pm on March 15, 2017: member

    Thanks @MarcoFalke . I’ve moved the new option to the general options section.

    New tip: https://github.com/jnewbery/bitcoin/tree/pr9966.3 / https://github.com/jnewbery/bitcoin/commit/d22d7b0958ccd8f7ed2f34563029388e606e2550

  20. Control mempool persistence using a command line parameter. 
    Mempool persistence was added in
3f78562df5e86a2a0a21812047fc3a7db8cee988, and is always on. This commit
introduces a command-line parameter -persistmempool, which defaults to
true. When set to false:
- mempool.dat is not loaded when the node starts.
- mempool.dat is not written when the node stops.
    91c91e140a
  21. jnewbery force-pushed on Mar 22, 2017
  22. jnewbery commented at 3:21 pm on March 22, 2017: member

    rebased https://github.com/jnewbery/bitcoin/tree/pr9966.3 -> https://github.com/jnewbery/bitcoin/tree/pr9966.4

    Are any reviewers able to reACK? All review comments are addressed, so this should be ready to merge.

  23. paveljanik commented at 3:46 pm on March 22, 2017: contributor
    I think that qa/rpc-tests no longer exists in the master.
  24. Add tests for mempool persistence 
    Adds tests for mempool persistence as well as for the new
-persistmempool command line parameter.
    a750d77b95
  25. jnewbery force-pushed on Mar 22, 2017
  26. jnewbery commented at 3:57 pm on March 22, 2017: member
    Thanks @paveljanik! Fixed
  28. in src/init.cpp:673 in a750d77b95 
    667@@ -666,8 +668,10 @@ void ThreadImport(std::vector<boost::filesystem::path> vImportFiles)
668         StartShutdown();
669     }
670     } // End scope of CImportingNow
671-    LoadMempool();
672-    fDumpMempoolLater = !fRequestShutdown;
673+    if (GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
674+        LoadMempool();
675+        fDumpMempoolLater = !fRequestShutdown;
    jtimon commented at 1:09 pm on March 25, 2017:
    You can do else fDumpMempoolLater = false to avoid the extra GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL) in Shutdown()
  29. jtimon commented at 1:10 pm on March 25, 2017: contributor
    Didn’t read the tests utACK a750d77
  30. laanwj merged this on May 3, 2017
  31. laanwj closed this on May 3, 2017
  32. laanwj referenced this in commit 2a183de0ec on May 3, 2017
  33. jnewbery deleted the branch on Jun 27, 2017
  34. PastaPastaPasta referenced this in commit 2dce41e998 on Jun 10, 2019
  35. PastaPastaPasta referenced this in commit 624ad8c53e on Jun 10, 2019
  36. PastaPastaPasta referenced this in commit 9b44353455 on Jun 10, 2019
  37. PastaPastaPasta referenced this in commit a019a6a600 on Jun 11, 2019
  38. PastaPastaPasta referenced this in commit b760028749 on Jun 11, 2019
  39. PastaPastaPasta referenced this in commit ce8a658c6a on Jun 15, 2019
  40. PastaPastaPasta referenced this in commit 783f2332fd on Jun 19, 2019
  41. PastaPastaPasta referenced this in commit c243207a72 on Jun 19, 2019
  42. PastaPastaPasta referenced this in commit 5bdfb393bc on Jun 19, 2019
  43. PastaPastaPasta referenced this in commit e5e34349a8 on Jun 19, 2019
  44. PastaPastaPasta referenced this in commit fe62f7c9a8 on Jun 19, 2019
  45. PastaPastaPasta referenced this in commit 80f8bb17dd on Jun 19, 2019
  46. PastaPastaPasta referenced this in commit fcc94adbe3 on Jun 19, 2019
  47. PastaPastaPasta referenced this in commit 2ed648a50b on Jun 20, 2019
  48. barrystyle referenced this in commit 7e640a758b on Jan 22, 2020
  49. furszy referenced this in commit 1f010c0969 on Jan 23, 2021
  50. MarcoFalke locked this on Sep 8, 2021


jnewbery luke-jr laanwj paveljanik TheBlueMatt MarcoFalke sipa jtimon

Labels
Mempool

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: 2024-07-03 13:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me