init: Stop indexes on shutdown after ChainStateFlushed callback. #17897

pull jimpo wants to merge 1 commits into bitcoin:master from jimpo:index-commit changing 1 files +8 −4
  1. jimpo commented at 8:12 PM on January 8, 2020: contributor

    Replaces #17852.

    Currently, the latest index state may not be committed to disk on shutdown. The state is committed on ChainStateFlushed callbacks and the current init order unregisters the indexes as validation interfaces before the final ChainStateFlushed callback is called on them.

    Issue identified by paulyc.

    For review: an alternative or supplemental solution would be to call Commit at the end of BaseIndex::Stop. I don't see any harm in doing so and it makes the less prone to user error. However, the destructor would have to be modified to not call Stop because Commit calls a virtual method, so I figured it wasn't worth it. But I'm curious how others feel.

  2. fanquake added the label UTXO Db and Indexes on Jan 8, 2020
  3. DrahtBot commented at 11:22 PM on January 8, 2020: 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:

    • #14053 (Add address-based index (attempt 4?) by marcinja)

    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.

  4. in src/init.cpp:247 in 4a6cb2f6bc outdated 
     243 | @@ -246,6 +244,10 @@ void Shutdown(NodeContext& node)
 244 |      // CValidationInterface callbacks, flush them...
 245 |      GetMainSignals().FlushBackgroundCallbacks();
 246 |  
 247 | +    // Stop all indexes only after flushing background callbacks.
    promag commented at 3:56 PM on January 9, 2020:

    nit, a similar comment could be added in BaseIndex::Stop.

    jimpo commented at 4:36 PM on January 9, 2020:

    Good point, though I think a better solution is to just call Commit from inside BaseIndex::Stop because I don't see much of a downside and it puts less of a burden on the caller.

  5. promag commented at 3:56 PM on January 9, 2020: member

    Code review ACK 4a6cb2f6bc7e27a8257ed475fb93f4a509bd2121.

  6. promag commented at 4:41 PM on January 9, 2020: member

    This is actually bad. The moved code is after

        g_txindex.reset();
    DestroyAllBlockFilterIndexes();

    which means the moved code does nothing now.

  7. init: Stop indexes on shutdown after ChainStateFlushed callback.
    Currently, the latest index state may not be committed to disk on shutdown.
    9dd58ca611
  8. jimpo force-pushed on Jan 10, 2020
  9. jimpo commented at 12:52 AM on January 10, 2020: contributor

    Thanks @promag, fixed.

  10. fanquake added the label Needs backport (0.19) on Jan 10, 2020
  11. jimpo force-pushed on Jan 10, 2020
  12. promag commented at 10:51 AM on January 10, 2020: member

    Looks good now, this time I'll test.

  13. promag commented at 3:43 PM on January 10, 2020: member

    Is it me or the following

        if (g_txindex) {
        g_txindex->Interrupt();
    }
    ForEachBlockFilterIndex([](BlockFilterIndex& index) { index.Interrupt(); });

    should be removed from Interrupt(NodeContext& node)?

  14. jimpo commented at 10:00 PM on January 10, 2020: contributor

    @promag I don't think so. Why?

  15. fanquake added this to the "Bugfixes" column in a project
  16. promag commented at 9:53 AM on January 14, 2020: member

    Actually I think it's fine, I was missing the !m_synced check.

    I was seeing this order of events:

    • Interrupt() is called which asynchronously stops the loop in ThreadSync()
    • before UnregisterValidationInterface is called, validation notifications can be delivered, which can't be handled.

    But now I see the m_synced flag to guard against this case.

    Anyway, would be nice to include steps/changes to reproduce the original problem.

    Code review ACK 9dd58ca611f6f2b59c25d727a4e955333525d345, but failed to test because I can't reproduce the original problem.

  17. paulyc commented at 5:27 AM on January 21, 2020: none

    Actually I think it's fine, I was missing the !m_synced check.

    I was seeing this order of events:

    • Interrupt() is called which asynchronously stops the loop in ThreadSync()
    • before UnregisterValidationInterface is called, validation notifications can be delivered, which can't be handled.

    But now I see the m_synced flag to guard against this case.

    Anyway, would be nice to include steps/changes to reproduce the original problem.

    Code review ACK 9dd58ca, but failed to test because I can't reproduce the original problem.

    So here is how I can currently reproduce it:

    Create a new datadir, start syncing with txindex=1. Let it run a while until there are decent number of block files, say to block 190000. Kill it. So when you restart, you'd expect that the txindex would restart at the same point as the block indexer at 190000 since they were both at the same point before you killed the daemon, but instead, I see the txindex starting at 39339, even though the data is in the txindex leveldb, it still has to run through every block again until it catches up and sets a new checkpoint, which is what led me to believe it wasn't being closed correctly since that's when leveldb writes out the current metadata for the next load. Here's my logfile from doing that bitcoind.txindex.redacted.log

  18. promag commented at 2:40 PM on January 21, 2020: member

    Thanks @paulyc, I have done that but didn't wait for that block count.

  19. fjahr commented at 12:09 AM on January 22, 2020: member

    tested ACK 9dd58ca611f6f2b59c25d727a4e955333525d345

    Tested as described by @paulyc. Also ran tests locally.

  20. kallewoof commented at 6:59 AM on January 22, 2020: member

    Tested ACK 9dd58ca611f6f2b59c25d727a4e955333525d345

    <details> <summary>master branch showing txindex sync begins at block 0 upon restart</summary>

    $ ./bitcoind -txindex=1 -datadir=oij
[...]
2020-01-22T06:11:44Z Shutdown: In progress...
2020-01-22T06:11:44Z net thread exit
2020-01-22T06:11:44Z UpdateTip: new best=000000000000035010292db86f97a207d96a4106af108fcb016550dee0e4f96e height=168089 version=0x00000001 log2_work=67.727578 tx=2483951 date='2012-02-23T10:53:35Z' progress=0.004994 cache=202.5MiB(1524617txo)
[...]
2020-01-22T06:12:21Z Shutdown: done
$ ./bitcoind -txindex=1 -datadir=oij
[...]
2020-01-22T06:12:56Z Opening LevelDB in /Users/me/workspace/bitcoin/src/oij/indexes/txindex
2020-01-22T06:12:57Z Opened LevelDB successfully
2020-01-22T06:12:57Z Using obfuscation key for /Users/me/workspace/bitcoin/src/oij/indexes/txindex: 0000000000000000
2020-01-22T06:12:57Z txindex thread start
2020-01-22T06:12:57Z init message: Loading wallet...
2020-01-22T06:12:57Z Syncing txindex with block chain from height 0
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]

    </details>

    <details> <summary>#17897 PR showing txindex sync prevails upon restart</summary>

    $ ./bitcoind -txindex=1 -datadir=oij-jimpo
[...]
2020-01-22T06:17:35Z Opening LevelDB in /Users/me/workspace/bitcoin/src/oij-jimpo/indexes/txindex
2020-01-22T06:17:35Z Opened LevelDB successfully
2020-01-22T06:17:35Z Using obfuscation key for /Users/me/workspace/bitcoin/src/oij-jimpo/indexes/txindex: 0000000000000000
2020-01-22T06:17:35Z txindex thread start
2020-01-22T06:17:35Z txindex is enabled
2020-01-22T06:17:35Z init message: Loading wallet...
2020-01-22T06:17:35Z txindex thread exit
[...]
2020-01-22T06:39:03Z UpdateTip: new best=000000000000000a92ff09eb56af38ad02b64ee75d4b71274caa437efc1e8b6c height=172386 version=0x00000001 log2_work=67.87869 tx=2670294 date='2012-03-22T19:16:30Z' progress=0.005368 cache=213.9MiB(1617994txo)
[...]
2020-01-22T06:39:40Z Shutdown: done
$ ./bitcoind -txindex=1 -datadir=oij-jimpo
[...]
2020-01-22T06:46:12Z Loaded best chain: hashBestChain=000000000000000a92ff09eb56af38ad02b64ee75d4b71274caa437efc1e8b6c height=172386 date=2012-03-22T19:16:30Z progress=0.005368
[...]
2020-01-22T06:46:15Z Opening LevelDB in /Users/me/workspace/bitcoin/src/oij-jimpo/indexes/txindex
2020-01-22T06:46:16Z Opened LevelDB successfully
2020-01-22T06:46:16Z Using obfuscation key for /Users/me/workspace/bitcoin/src/oij-jimpo/indexes/txindex: 0000000000000000
2020-01-22T06:46:16Z txindex thread start
2020-01-22T06:46:16Z txindex is enabled at height 172386
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2020-01-22T06:46:16Z txindex thread exit
[...]

    </details>

  21. fanquake commented at 9:29 AM on January 22, 2020: member

    Looks good. I've tested master (742f84d0de79fcaf17578f9c3376be248c3259bc) vs this PR (9dd58ca611f6f2b59c25d727a4e955333525d345) and am seeing the same fixed behaviour. Thanks @promag, @fjahr, @kallewoof and @paulyc for testing.

    master

    2020-01-22T08:56:22Z UpdateTip: new best=0000000000002d533ff0e59ee25496b4f403d8ce7b4b7fcd9181dadad8f625b3 height=118827 version=0x00000001 log2_work=61.774365 tx=416409 date='2011-04-17T13:39:54Z' progress=0.000837 cache=33.9MiB(212154txo)
^C2020-01-22T08:56:24Z tor: Thread interrupt
2020-01-22T08:56:24Z Shutdown: In progress...
<snip>
2020-01-22T08:56:25Z Shutdown: done
....
bitcoin $ ./src/bitcoind -txindex -datadir=/Users/michael/Desktop/master
2020-01-22T08:56:32Z Bitcoin Core version v0.19.99.0-742f84d0d (release build)
<snip>
2020-01-22T08:56:35Z Loaded best chain: hashBestChain=0000000000002d533ff0e59ee25496b4f403d8ce7b4b7fcd9181dadad8f625b3 height=118827 date=2011-04-17T13:39:54Z progress=0.000837
<snip>
2020-01-22T08:56:36Z Using obfuscation key for /Users/michael/Desktop/master/indexes/txindex: 0000000000000000
2020-01-22T08:56:36Z txindex thread start
2020-01-22T08:56:36Z Syncing txindex with block chain from height 65

    This PR

    2020-01-22T09:19:53Z Shutdown: In progress...
2020-01-22T09:19:53Z UpdateTip: new best=00000000000092c47111d9ca453b6df25b6e139faf438ab06070638417195fba height=116260 version=0x00000001 log2_work=61.415013 tx=385266 date='2011-04-02T06:16:42Z' progress=0.000774 cache=30.4MiB(196729txo)
<snip>
2020-01-22T09:20:10Z Shutdown: done
....
bitcoin $ ./src/bitcoind -txindex -datadir=/Users/michael/Desktop/17897
2020-01-22T09:20:54Z Bitcoin Core version v0.19.99.0-9dd58ca61 (release build)
<snip>
2020-01-22T09:21:02Z Loaded best chain: hashBestChain=00000000000092c47111d9ca453b6df25b6e139faf438ab06070638417195fba height=116260 date=2011-04-02T06:16:42Z progress=0.000774
<snip>
2020-01-22T09:21:07Z Using obfuscation key for /Users/michael/Desktop/17897/indexes/txindex: 0000000000000000
2020-01-22T09:21:07Z txindex thread start
2020-01-22T09:21:07Z init message: Loading wallet...
2020-01-22T09:21:07Z txindex is enabled at height 116260
  22. fanquake referenced this in commit a51aa2880d on Jan 22, 2020
  23. fanquake merged this on Jan 22, 2020
  24. fanquake closed this on Jan 22, 2020
  25. fanquake referenced this in commit 9d980c1f7f on Jan 22, 2020
  26. fanquake removed the label Needs backport (0.19) on Jan 22, 2020
  27. fanquake commented at 10:27 AM on January 22, 2020: member

    This has been backported to the 0.19 branch in 9d980c1f7f6fd0eb686d3cc94ba4698547f151c2.

  28. fanquake removed this from the "Bugfixes" column in a project
  29. sidhujag referenced this in commit 8c909d56ea on Jan 24, 2020
  30. MarkLTZ referenced this in commit 2e6abb1507 on Mar 13, 2020
  31. MarkLTZ referenced this in commit ece88bae51 on Mar 13, 2020
  32. sidhujag referenced this in commit 6271403c3d on Nov 10, 2020
  33. deadalnix referenced this in commit bba6f704d5 on Dec 14, 2020
  34. DrahtBot locked this on Feb 15, 2022
Contributors
jimpoDrahtBotpromagpaulycfjahrkallewooffanquake
Labels
UTXO Db and Indexes
Linked (view graph)
#17852 util: Close txindex LDB before exiting bitcoind
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-18 09:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me