index: initial sync speedup, parallelize process #26966

pull furszy wants to merge 6 commits into bitcoin:master from furszy:2022_parallelize_blockfilter_index_2 changing 16 files +577 −43
  1. furszy commented at 1:36 pm on January 25, 2023: member

    The current procedure for building the block filter index involves processing filters one at a time; Reading blocks, undo data, and previous headers from disk sequentially.

    This PR introduces a new mechanism to perform the work concurrently. Dividing the filters generation workload among a pool of workers that can be configured by the user, significantly increasing the speed of the index construction process.

    The same concurrent processing model has been applied to the transactions index as well.

    The newly introduced init flag -indexworkers=<n> enables the concurrent sync behavior. Where “n” is the number of worker threads that will be spawned at startup to create ranges of block filters during the initial sync process. Destroying the workers pool once the initial sync completes. Note: by default, the parallelized sync process is not enabled.

    Now the juicy part: In my computer, with the node in debug mode and on IBD, with -indexworkers=4, the block filter index generation took less than an hour. While, in master, the sync took more than 7 hours.

    Important Note: As the access to the block data on disk is protected by cs_main, this new feature runs substantially faster when the node is not in IBD.

    Footnote: This is part of a larger project :).

  2. DrahtBot commented at 1:36 pm on January 25, 2023: contributor

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

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/26966.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Concept ACK w0xlt, TheCharlatan

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30469 (index: Fix coinstats overflow and introduce index versioning by fjahr)

    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.

  3. DrahtBot added the label UTXO Db and Indexes on Jan 25, 2023
  4. furszy force-pushed on Jan 25, 2023
  5. furszy force-pushed on Jan 25, 2023
  6. furszy force-pushed on Jan 25, 2023
  7. Sjors commented at 2:38 pm on January 26, 2023: member
    Cool, will take it for a spin…
  8. furszy force-pushed on Jan 28, 2023
  9. furszy commented at 3:18 pm on January 28, 2023: member

    Cool @Sjors, just pushed a small update. Found a little bug.

    Going to add an important note to the PR description (because otherwise testing results will vary a lot):

    As the access to the block data on disk is protected by cs_main, this new feature runs substantially faster when the node is not in IBD. (where “substantially” here means full index sync, with 5 workers, in less than 20 minutes in my computer).

  10. furszy force-pushed on Jan 28, 2023
  11. mzumsande commented at 10:49 pm on January 29, 2023: contributor
    It’s probably much easier suggested than done, but did you attempt to implement parallelization in a more general way so that other indices could benefit from it as well? On first glance, txindex, and the indices suggested in PRs (#24539, #26951) seem to be parallelizable as well (not coinstatsindex though).
  12. furszy commented at 1:37 pm on January 30, 2023: member

    It’s probably much easier suggested than done, but did you attempt to implement parallelization in a more general way so that other indices could benefit from it as well?

    Yeah, that is part of the plan. I started with the block filter index because it requires an special treatment that txindex parallelization does not require (block/undo data reading and block filters creation can be parallelized but writing must be done sequentially due the need to link filter headers to their predecessors to create the filters-chain on disk).

    My idea was to start reviewing this one, so the process gets as clean as possible, and then move forward with the generalization step. It’s usually more natural to abstract processes when the specific cases are well-defined.

  13. furszy force-pushed on Jan 30, 2023
  14. w0xlt commented at 4:42 pm on January 30, 2023: contributor

    Concept ACK

    Perhaps it could have separate parallelization and index logic. So it could be reused in other indexes.

  15. furszy force-pushed on Jan 30, 2023
  16. furszy force-pushed on Jan 30, 2023
  17. Sjors commented at 1:47 pm on February 7, 2023: member

    Building the index on (AMD Ryzen 7950X, blocks stored on SSD):

    • master @ fe86616bb4ad0c4296d34299bc2e2f0fca1fe936: 35'15" (mostly 1 alternating CPU thread)
    • this PR (rebased)
      • n=8: 5'20" (uses about 8 of 32 CPU threads as expected)
      • n=32: 4'26" (pleasantly close to 100% CPU usage with a dip every 10 seconds, but it drops to only 1 CPU in the last minute or two)

    I made sure to not load any wallets and disabled other indexes.

    I didn’t test if the index was correct.

    I wonder if, for users without this index, it would be faster to generate the index, rescan the wallet and then delete it again. Combined with #26951 you would only have to generate filters up to the age of the wallet (IIUC, cc @pstratem).

    Note to self for future benchmarks: -txindex takes 1:07'14", -coinstatsindex takes 3.5 hours.

  18. furszy commented at 2:18 pm on February 7, 2023: member

    Great results @Sjors!.

    Could also give it a run rebased on top #27006. On master, the index initial sync is slower when the node is in IBD because the index thread has to compete for access to block data on disk through cs_main acquisition.

    I didn’t test if the index was correct.

    The PR contain a test verifying it.

    Side note: I’m working on generalizing the parallelization flow so other indexes, like the txindex and #26951 can make use of it too.

  19. DrahtBot added the label Needs rebase on Feb 17, 2023
  20. furszy force-pushed on Feb 21, 2023
  21. furszy force-pushed on Feb 21, 2023
  22. DrahtBot removed the label Needs rebase on Feb 21, 2023
  23. furszy force-pushed on Feb 22, 2023
  24. furszy force-pushed on Feb 27, 2023
  25. furszy renamed this:
    index: blockfilter initial sync speedup, parallelize process
    index: blockfilter and txindex initial sync speedup, parallelize process
    on Feb 27, 2023
  26. furszy renamed this:
    index: blockfilter and txindex initial sync speedup, parallelize process
    index: blockfilter initial sync speedup, parallelize process
    on Feb 27, 2023
  27. furszy force-pushed on Feb 27, 2023
  28. furszy commented at 12:40 pm on February 28, 2023: member

    PR updated, most of it implementation has changed.

    The news are:

    1. Decreased ThreadSync cs_main lock contention.
    2. Removed CBlockIndex access from the child indexes internals.
    3. Implemented generic workers pool.
    4. Introduced a last header cache for the Block Filter index. Avoiding disk reads on every new processed block.
    5. Enabled parallel sync on the tx index.

    Important Note: The introduced workers pool spawned by the -indexworkers init arg is shared among all the enabled indexes that support parallel sync.

    The implementation uses std::any mainly to simplify the patch-set, the base class template form of it requires a larger set of changes.

    Side note: in a first glance and without going too far over the coinstats index implementation, I would say that it could also be parallelized. But will leave it for a follow-up to not continue expanding the PR size.

    Future (doesn’t need to be included here, just mentioning so the path is clear): I’m working on decoupling the initial sync logic into a separate structure, so indexes subscribe to events instead of reading blocks from disk by themselves.

  29. DrahtBot added the label Needs rebase on May 11, 2023
  30. DrahtBot removed the label Needs rebase on May 11, 2023
  31. furszy force-pushed on May 11, 2023
  32. DrahtBot added the label Needs rebase on Jun 12, 2023
  33. furszy force-pushed on Jun 21, 2023
  34. DrahtBot removed the label Needs rebase on Jun 21, 2023
  35. DrahtBot added the label Needs rebase on Jun 30, 2023
  36. furszy force-pushed on Jun 30, 2023
  37. DrahtBot removed the label Needs rebase on Jun 30, 2023
  38. DrahtBot added the label CI failed on Jun 30, 2023
  39. DrahtBot removed the label CI failed on Jul 4, 2023
  40. DrahtBot added the label Needs rebase on Jul 6, 2023
  41. furszy force-pushed on Aug 9, 2023
  42. DrahtBot removed the label Needs rebase on Aug 9, 2023
  43. DrahtBot added the label CI failed on Aug 29, 2023
  44. furszy force-pushed on Aug 29, 2023
  45. DrahtBot commented at 8:34 pm on September 4, 2023: contributor
    Could mark as draft while CI is red?
  46. furszy force-pushed on Sep 4, 2023
  47. DrahtBot commented at 5:47 am on September 6, 2023: contributor
    It looks like tsan failed, but there is no log, when there should be a log. Maybe it was accidentally removed by #27667 ?
  48. DrahtBot added the label Needs rebase on Sep 12, 2023
  49. furszy force-pushed on Sep 12, 2023
  50. DrahtBot removed the label CI failed on Sep 12, 2023
  51. DrahtBot removed the label Needs rebase on Sep 12, 2023
  52. DrahtBot added the label Needs rebase on Oct 2, 2023
  53. furszy force-pushed on Oct 18, 2023
  54. DrahtBot removed the label Needs rebase on Oct 18, 2023
  55. DrahtBot added the label CI failed on Oct 18, 2023
  56. maflcko commented at 9:16 am on October 25, 2023: member
    CI is still red. Also, how would this work with AU?
  57. furszy force-pushed on Oct 27, 2023
  58. furszy force-pushed on Oct 27, 2023
  59. DrahtBot removed the label CI failed on Oct 27, 2023
  60. Sjors commented at 11:09 am on November 7, 2023: member

    Could also give it a run rebased on top #27006.

    That PR currently does not cleanly cherry-pick on top of this PR, not can I (trivially) rebase this PR on top top of it. Happy to try if you can make a branch.

    I just tried it again, deleting the blockfilterindex and rebuilding it. My impression is that it’s going slower than before and I’m not seeing much CPU activity.

    I also noticed the shutdown takes a very long time, with the indexer (?) threads sticking around for many minutes:

  61. in src/index/blockfilterindex.h:46 in 003f076a15 outdated 
    41@@ -42,6 +42,9 @@ class BlockFilterIndex final : public BaseIndex
42     /** cache of block hash to filter header, to avoid disk access when responding to getcfcheckpt. */
43     std::unordered_map<uint256, uint256, FilterHeaderHasher> m_headers_cache GUARDED_BY(m_cs_headers_cache);
44 
45+    // Last computed header to avoid disk reads at every new block.
46+    uint256 last_header{};
    TheCharlatan commented at 12:58 pm on November 24, 2023:
    NIt: Might take this opportunity to add the <uint256.h> header?
    furszy commented at 6:03 pm on March 22, 2024:
    hmm sorry, I missed to add it on #28955.
  62. in src/util/threadpool.h:58 in 46e74257f2 outdated 
    53+public:
54+    ThreadPool() {}
55+
56+    ~ThreadPool()
57+    {
58+        Stop(); // In case it hasn't being stopped.
    TheCharlatan commented at 3:18 pm on November 24, 2023:
    NIt: // In case it hasn't been stoppped.
    furszy commented at 6:03 pm on March 22, 2024:
    done as suggested. Thanks.
  63. in src/test/threadpool_tests.cpp:1 in 46e74257f2 outdated 
    0@@ -0,0 +1,158 @@
1+// Copyright (c) 2012-2022 The Bitcoin Core developers
    TheCharlatan commented at 3:19 pm on November 24, 2023:
    Nit: Adjust copyright date.
    furszy commented at 6:03 pm on March 22, 2024:
    done as suggested. Thanks.
  64. in src/util/threadpool.h:25 in 46e74257f2 outdated 
    11+
12+#include <condition_variable>
13+#include <future>
14+#include <queue>
15+#include <thread>
16+
    TheCharlatan commented at 3:32 pm on November 24, 2023:

    If I run IWYU locally, it reports the following headers as missing:

    0#include <cstddef>            // for size_t
1#include <algorithm>           // for max
2#include <atomic>              // for atomic
3#include <functional>          // for function
4#include <memory>              // for make_shared
5#include <stdexcept>           // for runtime_error
6#include <utility>             // for move, swap
7#include <vector>              // for vector
    furszy commented at 6:19 pm on March 22, 2024:
    done as suggested. Thanks.
  65. in src/util/threadpool.h:32 in 46e74257f2 outdated 
    19+private:
20+    Mutex cs_work_queue;
21+    std::queue<std::function<void()>> m_work_queue GUARDED_BY(cs_work_queue);
22+    std::condition_variable m_condition;
23+    // Stop indicator
24+    std::atomic<bool> m_stop{false};
    TheCharlatan commented at 10:37 pm on November 24, 2023:
    Just a comment: It would be nice if we could re-use the CThreadInterrupt interface here, but I don’t think it’s easily possible.
    furszy commented at 6:19 pm on March 22, 2024:
    Hmm, it shouldn’t be that hard, let me see.
  66. in src/util/threadpool.h:105 in 46e74257f2 outdated 
    94+        m_condition.notify_one();
95+        return future;
96+    }
97+
98+    // Synchronous processing
99+    void ProcessTask() EXCLUSIVE_LOCKS_REQUIRED(!cs_work_queue)
    TheCharlatan commented at 10:41 pm on November 24, 2023:
    Why is this added?
    furszy commented at 6:27 pm on March 22, 2024:

    Why is this added?

    Because I initially thought on pushing extra work into the thread pool queue so then the originator thread, once it finishes calculating the different tasks, can take the workload on its active wait. But I ended up implementing it differently to re-use the same piece of code for the single-thread approach (Can check it looking for where it says “// Otherwise, this is an active-wait, so we process blocks until all workers finish.”) Still, I think that having a method to process tasks manually make sense for a general thread pool implementation. But could remove it if you are strong on it.

    andrewtoth commented at 1:00 pm on October 26, 2024:
    This would make sense if we wanted to reuse this for CCheckQueue. Then, we could just loop and ProcessTask on the main thread once when we call Wait.
  67. in src/util/threadpool.h:56 in 46e74257f2 outdated 
    45+                m_work_queue.pop();
46+            }
47+
48+            // Execute the task without the lock
49+            WITH_REVERSE_LOCK(wait_lock, task());
50+        }
    TheCharlatan commented at 12:37 pm on November 25, 2023:
    I don’t think it would be necessary for the tasks themselves, since you already demonstrate in the tests how to retrieve exceptions, but I think some kind of exception handling and infomation logging similar to the one of util::TraceThread would still be nice. Did you choose not to use TraceThread on purpose?
    furszy commented at 6:37 pm on March 22, 2024:

    I don’t think it would be necessary for the tasks themselves, since you already demonstrate in the tests how to retrieve exceptions, but I think some kind of exception handling and infomation logging similar to the one of util::TraceThread would still be nice. Did you choose not to use TraceThread on purpose?

    I probably wasn’t aware of TraceThread when this was implemented. Changing it.. Thanks!

  68. in src/test/threadpool_tests.cpp:25 in 46e74257f2 outdated 
    19+    // 6) Busy workers, help them by processing tasks from outside.
20+
21+    // Test case 1, submit tasks and verify completion.
22+    {
23+        int num_workers = 3;
24+        int num_tasks = 50;
    TheCharlatan commented at 12:44 pm on November 25, 2023:
    Could use this test to stress test the pool a bit with more tasks?
    furszy commented at 6:55 pm on March 22, 2024:

    Could use this test to stress test the pool a bit with more tasks?

    What about creating a fuzzing test instead? I’m not sure about the benefits of adding more tasks here. I can only foresee other developers complaining about the increased unit test times.

  69. in src/test/threadpool_tests.cpp:29 in 46e74257f2 outdated 
    24+        int num_tasks = 50;
25+
26+        ThreadPool threadPool;
27+        threadPool.Start(num_workers);
28+        std::atomic<int> counter = 0;
29+        for (int i=0; i<num_tasks; i++) {
    TheCharlatan commented at 12:45 pm on November 25, 2023:

    I think a test where each task is assigned slightly different data would be nice. Something like:

    0+        std::atomic<int> par_sum{0};
1+        for (int i = 0; i < num_tasks; i++) {
2+            threadPool.Submit([&par_sum,i]() {
3+                par_sum += i;
4             });
5         }
6+        int sync_sum{0};
7+        for (int i = 0; i < num_tasks; i++) {
8+            sync_sum += i;
9+        }
    furszy commented at 11:48 am on March 23, 2024:
    Sure. Thats the Gauss sum :).
  70. TheCharlatan commented at 12:54 pm on November 25, 2023: contributor
    This all looks pretty promising. I left some feedback before continuing with the latter half of the PR.
  71. in src/index/base.h:29 in 7b2d4d471a outdated 
    23 namespace Consensus {
24     struct Params;
25 }
26 
27+/** Number of concurrent jobs during the initial sync process */
28+const int16_t INDEX_WORKERS_COUNT = 0;
    TheCharlatan commented at 4:22 pm on November 25, 2023:
    static constexpr int16_t INDEX_WORKERS_COUNT{0} (and similarly below)?
    furszy commented at 12:45 pm on March 23, 2024:
    sure
  72. in src/init.cpp:1984 in 7b2d4d471a outdated 
    1980+    }
1981+
1982     // Start threads
1983-    for (auto index : node.indexes) if (!index->StartBackgroundSync()) return false;
1984+    for (auto index : node.indexes) {
1985+        // todo: Only provide thread pool to indexes that supports parallel sync
    TheCharlatan commented at 4:46 pm on November 25, 2023:
    In commit 7b2d4d471ae251d4c22184dda26b73993a65eff8: Could the AllowParallelSync method be moved to this commit?
    furszy commented at 12:53 pm on March 23, 2024:

    In commit 7b2d4d4: Could the AllowParallelSync method be moved to this commit?

    Sure.

  73. in src/index/base.cpp:229 in 727be9d500 outdated 
    226+            const CBlockIndex* it_start = pindex_next;
227+
228+            if (parallel_sync_enabled) {
229+                int max_blocks_to_sync = m_tasks_per_worker * m_thread_pool->WorkersCount() + m_tasks_per_worker; // extra 'm_tasks_per_worker' due the active-wait.
230+                int tip_height = WITH_LOCK(cs_main, return m_chainstate->m_chain.Height());
231+                int remaining_blocks = tip_height - pindex_next->nHeight;
    TheCharlatan commented at 5:25 pm on November 25, 2023:
    Make max_blocks_to_sync, tip_height, and remaining_blocks const.
    furszy commented at 12:54 pm on March 23, 2024:
    Done as suggested.
  74. in src/index/base.cpp:245 in 727be9d500 outdated 
    230+                int tip_height = WITH_LOCK(cs_main, return m_chainstate->m_chain.Height());
231+                int remaining_blocks = tip_height - pindex_next->nHeight;
232+                work_chunk = remaining_blocks > max_blocks_to_sync ? m_tasks_per_worker : remaining_blocks / (m_thread_pool->WorkersCount() + 1);
233+                workers_count = m_thread_pool->WorkersCount();
234+                if (work_chunk == 0) { // disable parallel sync if we are close to the tip
235+                    workers_count = 0;
    TheCharlatan commented at 5:28 pm on November 25, 2023:
    Nitty nit: Keep the order of workers_count and work_chunk consistent?
    furszy commented at 12:55 pm on March 23, 2024:

    Nitty nit: Keep the order of workers_count and work_chunk consistent?

    What do you mean? Set workers_count first, then work_chunk always in the same order?

  75. in src/index/base.cpp:247 in 727be9d500 outdated 
    244+                    it_start = WITH_LOCK(::cs_main, return NextSyncBlock(it_end, m_chainstate->m_chain));
245+                }
246+            }
247 
248+            // If we have only one block to process, run it directly.
249+            // Otherwise, this is an active-wait, so we process blocks until all workers finish.
    TheCharlatan commented at 10:38 pm on November 25, 2023:
    Would so we also process blocks in this thread until all workers finish. be more accurate?
    furszy commented at 12:56 pm on March 23, 2024:
    Sure. Done as suggested. Thanks.
  76. in src/test/blockfilter_index_tests.cpp:14 in de69506c09 outdated 
    13@@ -14,6 +14,7 @@
14 #include <test/util/blockfilter.h>
    TheCharlatan commented at 10:52 pm on November 25, 2023:
    In commit de69506c090f530f34d78478db39898bd4e2cbba: I think the test introduction should be squashed into the following commit, or do you want to demonstrate something here first?
    furszy commented at 1:01 pm on March 23, 2024:

    In commit de69506: I think the test introduction should be squashed into the following commit, or do you want to demonstrate something here first?

    hmm, will squash them. I don’t remember why I split them (2 years ago).

  77. TheCharlatan commented at 11:01 pm on November 25, 2023: contributor

    Concept ACK.

    Done with my first pass, still want to think some of the approaches here over a bit.

  78. in src/init.cpp:2085 in 373b0bb077 outdated 
    1974+    if (node.args->IsArgSet("-indexworkers")) {
1975+        int index_workers = node.args->GetIntArg("-indexworkers", INDEX_WORKERS_COUNT);
1976+        if (index_workers < 0 || index_workers > 100) return InitError(_("Invalid -indexworkers arg"));
1977+
1978+        thread_pool = std::make_shared<ThreadPool>();
1979+        thread_pool->Start(index_workers);
    TheCharlatan commented at 4:08 pm on November 26, 2023:
    Since the constructor and Start are always called in succession in this patch set, you could make ThreadPool more RAII styled if the Start method were removed and instead placed in the constructor. Could also make sense to do the same with the destructor and Stop.
  79. DrahtBot added the label Needs rebase on Dec 14, 2023
  80. furszy commented at 3:22 pm on February 5, 2024: member
    Focus is on #28955, which contains a good number of commits decoupled from this PR. Will come back here after it.
  81. achow101 referenced this in commit 0b96a1925e on Mar 20, 2024
  82. furszy force-pushed on Mar 20, 2024
  83. DrahtBot removed the label Needs rebase on Mar 20, 2024
  84. DrahtBot added the label CI failed on Mar 21, 2024
  85. DrahtBot commented at 1:36 am on March 21, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/22906894781

  86. furszy force-pushed on Mar 21, 2024
  87. Sjors commented at 5:44 pm on March 21, 2024: member

    Running it again, let’s see how quick it is…

    Since I might set indexworkers=32 in my config file, which is great for the initial sync and if it needs to do a big catchup. But does it cause much overhead when it’s up to date? Maybe the threads should spin down if there’s not much work.

    What happens when there are multiple readBlockFromDisk calls around the same time? I don’t see any (obvious) thread locking happening in CAutoFile. Since I keep block files on a spinning disk (-blocksdir), I wonder if that potentially slows things down - compared to fetching one file in a single uninterrupted operation.

    So far (block 200K) my spinning disk is making a ton of noise and CPU activity is negligible.

    It took 4 hours and 23 minutes. That’s an improvement over the 5 hours 46 minutes without: #28955 (comment)

    Note that last year I tested with SSD - which resulted in a 16x improvement #26966 (comment). This time I used a spinning disk. CPU activity was negligible all the way.

  88. furszy commented at 5:54 pm on March 21, 2024: member

    Running it again, let’s see how quick it is…

    Since I might set indexworkers=32 in my config file, which is great for the initial sync and if it needs to do a big catchup. But does it cause much overhead when it’s up to date? Maybe the threads should spin down if there’s not much work.

    Yeah sure. The thread pool can be destructed once all indexes initial sync finish (once the index is synced, it starts receiving blocks through the validation signals and does not use the initial sync workers anymore). I’m currently tackling theCharlatan’s feedback, and thinking about some improvements. Will add this change too on the next push.

  89. DrahtBot removed the label CI failed on Mar 21, 2024
  90. furszy force-pushed on Mar 23, 2024
  91. furszy commented at 1:34 pm on March 23, 2024: member

    Thanks for the in-depth review theCharlatan! Most comments were tackled. And thanks for testing Sjors!

    Now that we’ve reached this point (after #28955 merge), I’m rethinking and polishing the design. I’m not totally convinced about the current implementation anymore. We’ve grown a lot since this was implemented two years ago. Other than that, Sjors had a nice idea that I want to try out (or at least design this in a way so that the idea can be implemented in isolation in the future) –> Instead of dividing the work based on block ranges, it can be divided based on block file ranges. This would minimize the needle movement on spinning disks.

  92. DrahtBot added the label CI failed on Mar 23, 2024
  93. DrahtBot commented at 2:10 pm on March 23, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/23011013574

  94. Sjors commented at 9:06 am on March 25, 2024: member

    Instead of dividing the work based on block ranges, it can be divided based on block file ranges. This would minimize the needle movement on spinning disks.

    It’s possible that this can be achieved with block ranges too. But you have to make sure only one block range is read at any given time. I.e. the other threads should wait while a disk read is in progress. I suspect the problem lies in having 32 threads trying to read different things at the same time, and then operating system goes and fetches a few kilobytes, a few kilobytes there, etc. Though I haven’t measured this.

  95. DrahtBot removed the label CI failed on Apr 4, 2024
  96. DrahtBot commented at 7:54 am on April 5, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/23011013574

  97. DrahtBot added the label CI failed on Apr 5, 2024
  98. DrahtBot commented at 7:59 am on April 5, 2024: contributor 
      0test/threadpool_tests.cpp(9): Entering test suite "threadpool_tests"
  1test/threadpool_tests.cpp(11): Entering test case "threadpool_basic"
  2==================
  3WARNING: ThreadSanitizer: data race (pid=26928)
  4  Write of size 8 at 0x721000012718 by thread T13 (mutexes: write M0):
  5    [#0](/bitcoin-bitcoin/0/) free <null> (test_bitcoin+0x156ae3) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
  6    [#1](/bitcoin-bitcoin/1/) std::range_error::~range_error() <null> (libc++abi.so.1+0x28b74) (BuildId: 40d8c515ee7a3c2d826acc730982f279ffe00146)
  7    [#2](/bitcoin-bitcoin/2/) std::__1::__shared_count::__release_shared[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:157:7 (test_bitcoin+0xb2e070) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
  8    [#3](/bitcoin-bitcoin/3/) std::__1::__shared_weak_count::__release_shared[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:186:25 (test_bitcoin+0xb2e070)
  9    [#4](/bitcoin-bitcoin/4/) std::__1::shared_ptr<std::__1::packaged_task<void ()>>::~shared_ptr[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/shared_ptr.h:648:17 (test_bitcoin+0xb2e070)
 10    [#5](/bitcoin-bitcoin/5/) std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'()::~() src/./util/threadpool.h:96:34 (test_bitcoin+0xb2e070)
 11    [#6](/bitcoin-bitcoin/6/) std::__1::__compressed_pair_elem<std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'(), 0, false>::~__compressed_pair_elem() /usr/lib/llvm-18/bin/../include/c++/v1/__memory/compressed_pair.h:44:8 (test_bitcoin+0xb2e070)
 12    [#7](/bitcoin-bitcoin/7/) std::__1::__function::__alloc_func<std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'(), std::__1::allocator<std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'()>, void ()>::destroy[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:182:58 (test_bitcoin+0xb2e070)
 13    [#8](/bitcoin-bitcoin/8/) std::__1::__function::__func<std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'(), std::__1::allocator<std::__1::future<decltype(fp())> ThreadPool::Submit<threadpool_tests::threadpool_basic::test_method()::$_6>(threadpool_tests::threadpool_basic::test_method()::$_6)::'lambda'()>, void ()>::destroy() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:297:8 (test_bitcoin+0xb2e070)
 14    [#9](/bitcoin-bitcoin/9/) std::__1::__function::__value_func<void ()>::~__value_func[abi:ne180100]() src/./sync.h (test_bitcoin+0x442c8b) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 15    [#10](/bitcoin-bitcoin/10/) std::__1::function<void ()>::~function() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:972:43 (test_bitcoin+0x442c8b)
 16    [#11](/bitcoin-bitcoin/11/) ThreadPool::WorkerThread() src/./util/threadpool.h:56:9 (test_bitcoin+0x442c8b)
 17    [#12](/bitcoin-bitcoin/12/) ThreadPool::Start(int)::'lambda'()::operator()() const src/./util/threadpool.h:73:101 (test_bitcoin+0x442ae5) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 18    [#13](/bitcoin-bitcoin/13/) decltype(std::declval<ThreadPool::Start(int)::'lambda'()&>()()) std::__1::__invoke[abi:ne180100]<ThreadPool::Start(int)::'lambda'()&>(ThreadPool::Start(int)::'lambda'()&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:344:25 (test_bitcoin+0x442ae5)
 19    [#14](/bitcoin-bitcoin/14/) void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ne180100]<ThreadPool::Start(int)::'lambda'()&>(ThreadPool::Start(int)::'lambda'()&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:419:5 (test_bitcoin+0x442ae5)
 20    [#15](/bitcoin-bitcoin/15/) std::__1::__function::__alloc_func<ThreadPool::Start(int)::'lambda'(), std::__1::allocator<ThreadPool::Start(int)::'lambda'()>, void ()>::operator()[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:169:12 (test_bitcoin+0x442ae5)
 21    [#16](/bitcoin-bitcoin/16/) std::__1::__function::__func<ThreadPool::Start(int)::'lambda'(), std::__1::allocator<ThreadPool::Start(int)::'lambda'()>, void ()>::operator()() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:311:10 (test_bitcoin+0x442ae5)
 22    [#17](/bitcoin-bitcoin/17/) std::__1::__function::__value_func<void ()>::operator()[abi:ne180100]() const /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:428:12 (test_bitcoin+0x1815b98) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 23    [#18](/bitcoin-bitcoin/18/) std::__1::function<void ()>::operator()() const /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:981:10 (test_bitcoin+0x1815b98)
 24    [#19](/bitcoin-bitcoin/19/) util::TraceThread(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>) src/util/thread.cpp:21:9 (test_bitcoin+0x1815b98)
 25    [#20](/bitcoin-bitcoin/20/) decltype(std::declval<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>)>()(std::declval<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>(), std::declval<ThreadPool::Start(int)::'lambda'()>())) std::__1::__invoke[abi:ne180100]<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:344:25 (test_bitcoin+0x44261e) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 26    [#21](/bitcoin-bitcoin/21/) void std::__1::__thread_execute[abi:ne180100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:193:3 (test_bitcoin+0x44261e)
 27    [#22](/bitcoin-bitcoin/22/) void* std::__1::__thread_proxy[abi:ne180100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>>(void*) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:202:3 (test_bitcoin+0x44261e)
 28  Previous read of size 8 at 0x721000012718 by main thread:
 29    [#0](/bitcoin-bitcoin/0/) strcmp <null> (test_bitcoin+0x161718) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 30    [#1](/bitcoin-bitcoin/1/) boost::test_tools::tt_detail::equal_impl(char const*, char const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/test_tools.ipp:463:30 (test_bitcoin+0x1fca6f) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 31    [#2](/bitcoin-bitcoin/2/) boost::test_tools::assertion_result boost::test_tools::tt_detail::equal_impl_frwd::call_impl<char const*, char [25]>(char const* const&, char const (&) [25], mpl_::bool_<false>) const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/tools/old/impl.hpp:130:16 (test_bitcoin+0xb2f88f) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 32    [#3](/bitcoin-bitcoin/3/) boost::test_tools::assertion_result boost::test_tools::tt_detail::equal_impl_frwd::operator()<char const*, char [25]>(char const* const&, char const (&) [25]) const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/tools/old/impl.hpp:145:16 (test_bitcoin+0xb2f88f)
 33    [#4](/bitcoin-bitcoin/4/) bool boost::test_tools::tt_detail::check_frwd<boost::test_tools::tt_detail::equal_impl_frwd, char const*, char [25]>(boost::test_tools::tt_detail::equal_impl_frwd, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, char const* const&, char const*, char const (&) [25], char const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/tools/old/impl.hpp:92:1 (test_bitcoin+0xb2f88f)
 34    [#5](/bitcoin-bitcoin/5/) threadpool_tests::threadpool_basic::test_method() src/test/threadpool_tests.cpp:117:13 (test_bitcoin+0xb2855b) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 35    [#6](/bitcoin-bitcoin/6/) threadpool_tests::threadpool_basic_invoker() src/test/threadpool_tests.cpp:11:1 (test_bitcoin+0xb26256) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 36    [#7](/bitcoin-bitcoin/7/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x31b9dd) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 37    [#8](/bitcoin-bitcoin/8/) boost::function0<void>::operator()() const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x26f5f8) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 38    [#9](/bitcoin-bitcoin/9/) boost::detail::forward::operator()() /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 (test_bitcoin+0x26f5f8)
 39    [#10](/bitcoin-bitcoin/10/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x26f5f8)
 40    [#11](/bitcoin-bitcoin/11/) boost::function0<int>::operator()() const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1f49f3) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 41    [#12](/bitcoin-bitcoin/12/) int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()>>(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30 (test_bitcoin+0x1f49f3)
 42    [#13](/bitcoin-bitcoin/13/) boost::execution_monitor::catch_signals(boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16 (test_bitcoin+0x1f49f3)
 43    [#14](/bitcoin-bitcoin/14/) boost::execution_monitor::execute(boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16 (test_bitcoin+0x1f4d6a) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 44    [#15](/bitcoin-bitcoin/15/) boost::execution_monitor::vexecute(boost::function<void ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5 (test_bitcoin+0x1f04d8) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 45    [#16](/bitcoin-bitcoin/16/) boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1f04d8)
 46    [#17](/bitcoin-bitcoin/17/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44 (test_bitcoin+0x227215) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 47    [#18](/bitcoin-bitcoin/18/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x227aa1) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 48    [#19](/bitcoin-bitcoin/19/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x227aa1) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 49    [#20](/bitcoin-bitcoin/20/) boost::unit_test::framework::run(unsigned long, bool) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1722:29 (test_bitcoin+0x1ef05f) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 50    [#21](/bitcoin-bitcoin/21/) boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9 (test_bitcoin+0x209e26) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 51    [#22](/bitcoin-bitcoin/22/) main /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12 (test_bitcoin+0x20a753) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 52  Mutex M0 (0x7fff25c67fe0) created at:
 53    [#0](/bitcoin-bitcoin/0/) pthread_mutex_trylock <null> (test_bitcoin+0x159aae) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 54    [#1](/bitcoin-bitcoin/1/) std::__1::mutex::try_lock() <null> (libc++.so.1+0x5f63c) (BuildId: b52c260d7d8ecb73699f2407c0a9b193fc4761fc)
 55    [#2](/bitcoin-bitcoin/2/) UniqueLock<AnnotatedMixin<std::__1::mutex>>::UniqueLock(AnnotatedMixin<std::__1::mutex>&, char const*, char const*, int, bool) src/./sync.h:182:13 (test_bitcoin+0x442c37) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 56    [#3](/bitcoin-bitcoin/3/) ThreadPool::WorkerThread() src/./util/threadpool.h:37:9 (test_bitcoin+0x442c37)
 57    [#4](/bitcoin-bitcoin/4/) ThreadPool::Start(int)::'lambda'()::operator()() const src/./util/threadpool.h:73:101 (test_bitcoin+0x442ae5) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 58    [#5](/bitcoin-bitcoin/5/) decltype(std::declval<ThreadPool::Start(int)::'lambda'()&>()()) std::__1::__invoke[abi:ne180100]<ThreadPool::Start(int)::'lambda'()&>(ThreadPool::Start(int)::'lambda'()&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:344:25 (test_bitcoin+0x442ae5)
 59    [#6](/bitcoin-bitcoin/6/) void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ne180100]<ThreadPool::Start(int)::'lambda'()&>(ThreadPool::Start(int)::'lambda'()&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:419:5 (test_bitcoin+0x442ae5)
 60    [#7](/bitcoin-bitcoin/7/) std::__1::__function::__alloc_func<ThreadPool::Start(int)::'lambda'(), std::__1::allocator<ThreadPool::Start(int)::'lambda'()>, void ()>::operator()[abi:ne180100]() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:169:12 (test_bitcoin+0x442ae5)
 61    [#8](/bitcoin-bitcoin/8/) std::__1::__function::__func<ThreadPool::Start(int)::'lambda'(), std::__1::allocator<ThreadPool::Start(int)::'lambda'()>, void ()>::operator()() /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:311:10 (test_bitcoin+0x442ae5)
 62    [#9](/bitcoin-bitcoin/9/) std::__1::__function::__value_func<void ()>::operator()[abi:ne180100]() const /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:428:12 (test_bitcoin+0x1815b98) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 63    [#10](/bitcoin-bitcoin/10/) std::__1::function<void ()>::operator()() const /usr/lib/llvm-18/bin/../include/c++/v1/__functional/function.h:981:10 (test_bitcoin+0x1815b98)
 64    [#11](/bitcoin-bitcoin/11/) util::TraceThread(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>) src/util/thread.cpp:21:9 (test_bitcoin+0x1815b98)
 65    [#12](/bitcoin-bitcoin/12/) decltype(std::declval<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>)>()(std::declval<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>>(), std::declval<ThreadPool::Start(int)::'lambda'()>())) std::__1::__invoke[abi:ne180100]<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__type_traits/invoke.h:344:25 (test_bitcoin+0x44261e) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 66    [#13](/bitcoin-bitcoin/13/) void std::__1::__thread_execute[abi:ne180100]<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:193:3 (test_bitcoin+0x44261e)
 67    [#14](/bitcoin-bitcoin/14/) void* std::__1::__thread_proxy[abi:ne180100]<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct>>, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>>(void*) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:202:3 (test_bitcoin+0x44261e)
 68  Thread T13 'b-threadpool_wo' (tid=26954, running) created by main thread at:
 69    [#0](/bitcoin-bitcoin/0/) pthread_create <null> (test_bitcoin+0x157daf) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 70    [#1](/bitcoin-bitcoin/1/) std::__1::__libcpp_thread_create[abi:ne180100](unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-18/bin/../include/c++/v1/__threading_support:317:10 (test_bitcoin+0x442473) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 71    [#2](/bitcoin-bitcoin/2/) std::__1::thread::thread<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), void>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__thread/thread.h:212:14 (test_bitcoin+0x442473)
 72    [#3](/bitcoin-bitcoin/3/) std::__1::thread* std::__1::construct_at[abi:ne180100]<std::__1::thread, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), std::__1::thread*>(std::__1::thread*, void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/construct_at.h:41:46 (test_bitcoin+0x4421c5) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 73    [#4](/bitcoin-bitcoin/4/) std::__1::thread* std::__1::__construct_at[abi:ne180100]<std::__1::thread, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), std::__1::thread*>(std::__1::thread*, void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/construct_at.h:49:10 (test_bitcoin+0x4421c5)
 74    [#5](/bitcoin-bitcoin/5/) void std::__1::allocator_traits<std::__1::allocator<std::__1::thread>>::construct[abi:ne180100]<std::__1::thread, void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'(), void, void>(std::__1::allocator<std::__1::thread>&, std::__1::thread*, void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/__memory/allocator_traits.h:305:5 (test_bitcoin+0x4421c5)
 75    [#6](/bitcoin-bitcoin/6/) std::__1::thread* std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::__emplace_back_slow_path<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/vector:1491:3 (test_bitcoin+0x4421c5)
 76    [#7](/bitcoin-bitcoin/7/) std::__1::thread& std::__1::vector<std::__1::thread, std::__1::allocator<std::__1::thread>>::emplace_back<void (*)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>, ThreadPool::Start(int)::'lambda'()>(void (*&&)(std::__1::basic_string_view<char, std::__1::char_traits<char>>, std::__1::function<void ()>), std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>&&, ThreadPool::Start(int)::'lambda'()&&) /usr/lib/llvm-18/bin/../include/c++/v1/vector:1511:13 (test_bitcoin+0x43efb4) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 77    [#8](/bitcoin-bitcoin/8/) ThreadPool::Start(int) src/./util/threadpool.h:73:23 (test_bitcoin+0x43efb4)
 78    [#9](/bitcoin-bitcoin/9/) threadpool_tests::threadpool_basic::test_method() src/test/threadpool_tests.cpp:109:20 (test_bitcoin+0xb283a6) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 79    [#10](/bitcoin-bitcoin/10/) threadpool_tests::threadpool_basic_invoker() src/test/threadpool_tests.cpp:11:1 (test_bitcoin+0xb26256) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 80    [#11](/bitcoin-bitcoin/11/) boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x31b9dd) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 81    [#12](/bitcoin-bitcoin/12/) boost::function0<void>::operator()() const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x26f5f8) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 82    [#13](/bitcoin-bitcoin/13/) boost::detail::forward::operator()() /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1388:32 (test_bitcoin+0x26f5f8)
 83    [#14](/bitcoin-bitcoin/14/) boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x26f5f8)
 84    [#15](/bitcoin-bitcoin/15/) boost::function0<int>::operator()() const /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1f49f3) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 85    [#16](/bitcoin-bitcoin/16/) int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()>>(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:301:30 (test_bitcoin+0x1f49f3)
 86    [#17](/bitcoin-bitcoin/17/) boost::execution_monitor::catch_signals(boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:903:16 (test_bitcoin+0x1f49f3)
 87    [#18](/bitcoin-bitcoin/18/) boost::execution_monitor::execute(boost::function<int ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1301:16 (test_bitcoin+0x1f4d6a) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 88    [#19](/bitcoin-bitcoin/19/) boost::execution_monitor::vexecute(boost::function<void ()> const&) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1397:5 (test_bitcoin+0x1f04d8) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 89    [#20](/bitcoin-bitcoin/20/) boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1f04d8)
 90    [#21](/bitcoin-bitcoin/21/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:815:44 (test_bitcoin+0x227215) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 91    [#22](/bitcoin-bitcoin/22/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x227aa1) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 92    [#23](/bitcoin-bitcoin/23/) boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:784:58 (test_bitcoin+0x227aa1) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 93    [#24](/bitcoin-bitcoin/24/) boost::unit_test::framework::run(unsigned long, bool) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1722:29 (test_bitcoin+0x1ef05f) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 94    [#25](/bitcoin-bitcoin/25/) boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:250:9 (test_bitcoin+0x209e26) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 95    [#26](/bitcoin-bitcoin/26/) main /ci_container_base/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:306:12 (test_bitcoin+0x20a753) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d)
 96SUMMARY: ThreadSanitizer: data race (/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/test_bitcoin+0x156ae3) (BuildId: 5ce48731414b473abd5838c79ce7029f5872bd2d) in free
 97==================
 98Running tests: translation_tests from test/translation_tests.cpp
 99make[3]: *** [Makefile:22558: test/threadpool_tests.cpp.test] Error 1
100make[3]: *** Waiting for unfinished jobs....
101make[3]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
102make[2]: *** [Makefile:20520: check-am] Error 2
103make[2]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
104make[1]: *** [Makefile:20185: check-recursive] Error 1
105make[1]: Leaving directory '/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
106make: *** [Makefile:756: check-recursive] Error 1
107Exit status: 2
  99. DrahtBot removed the label CI failed on Apr 5, 2024
  100. DrahtBot commented at 8:37 am on April 5, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/23011013574

  101. DrahtBot added the label CI failed on Apr 5, 2024
  102. DrahtBot removed the label CI failed on Apr 5, 2024
  103. DrahtBot added the label CI failed on Apr 5, 2024
  104. furszy force-pushed on Apr 7, 2024
  105. DrahtBot commented at 5:19 pm on April 17, 2024: contributor
    Are you still working on this? If not, this could be moved to draft for as long as the tsan CI is failing.
  106. furszy marked this as a draft on Apr 18, 2024
  107. DrahtBot added the label Needs rebase on Apr 30, 2024
  108. furszy force-pushed on May 1, 2024
  109. DrahtBot removed the label Needs rebase on May 1, 2024
  110. DrahtBot removed the label CI failed on May 1, 2024
  111. DrahtBot added the label CI failed on Jun 14, 2024
  112. DrahtBot commented at 2:16 pm on June 14, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/24470829129

  113. furszy force-pushed on Jun 26, 2024
  114. DrahtBot added the label Needs rebase on Aug 5, 2024
  115. furszy renamed this:
    index: blockfilter initial sync speedup, parallelize process
    index: initial sync speedup, parallelize process
    on Aug 7, 2024
  116. furszy force-pushed on Aug 14, 2024
  117. furszy force-pushed on Aug 14, 2024
  118. DrahtBot removed the label Needs rebase on Aug 14, 2024
  119. hebasto added the label Needs CMake port on Aug 16, 2024
  120. maflcko removed the label Needs CMake port on Aug 29, 2024
  121. DrahtBot added the label Needs rebase on Sep 2, 2024
  122. index: remove CBlockIndex access from node internals 
    Moved CBlockUndo disk read lookups from child
indexes to base index class.
    544ee5af8f
  123. util: introduce general purpose thread pool 401f21bfd7
  124. init: provide thread pool to block filter index 
    And add option to customize thread pool workers count
    32dffe6d06
  125. index: implement index parallel sync 5d0720e6ab
  126. index: enable block filter index parallel sync 
    It also adds coverage for initial sync from a particular block.
Mimicking a node restart.
    86f76df878
  127. txindex: enable parallel sync 81dcee9464
  128. furszy force-pushed on Oct 1, 2024
  129. DrahtBot removed the label Needs rebase on Oct 1, 2024
  130. in src/util/threadpool.h:45 in 81dcee9464 
    40+            {
41+                // Wait for the task or until the stop flag is set
42+                m_condition.wait(wait_lock,[&]() EXCLUSIVE_LOCKS_REQUIRED(cs_work_queue) { return m_interrupt || !m_work_queue.empty(); });
43+
44+                // If stopped, exit worker.
45+                if (m_interrupt && m_work_queue.empty()) {
    andrewtoth commented at 2:54 pm on October 23, 2024:
    If we receive an interrupt, we don’t also want to wait for the work queue to be empty right?
    andrewtoth commented at 4:52 pm on October 27, 2024:
    I tried this and it causes memory errors, since the remaining futures will have a dangling ref to m_condition.
  131. in src/util/threadpool.h:39 in 81dcee9464 
    34+
35+    void WorkerThread() EXCLUSIVE_LOCKS_REQUIRED(!cs_work_queue)
36+    {
37+        WAIT_LOCK(cs_work_queue, wait_lock);
38+        while (!m_interrupt) {
39+            std::function<void()> task;
    andrewtoth commented at 3:02 pm on October 23, 2024:
    Can we modify this to be more generic and return a type? And add logic to collect all returned values into a shared vector which can then be atomically swapped out by an observer? Possibly not in this PR, but if this will be split out into a generic thread pool.
  132. in src/util/threadpool.h:60 in 81dcee9464 
    55+            WITH_REVERSE_LOCK(wait_lock, task());
56+        }
57+    }
58+
59+public:
60+    ThreadPool() {}
    andrewtoth commented at 3:37 pm on November 18, 2024:

    tidy doesn’t like this

    0    ThreadPool() = default;
  133. andrewtoth commented at 6:11 pm on November 18, 2024: contributor

    I’m using the ThreadPool here in #31132 as a cherry-picked commit, modulo changing ThreadPool() {} to ThreadPool() = default;. Perhaps we could pull this out to a separate PR since it would be useful for both changes.

    One request for the ThreadPool would be to track in flight tasks being executed. That way we could write tests that ensure that all tasks have been completed before continuing, even if we don’t have access to the futures.


furszy DrahtBot Sjors mzumsande w0xlt maflcko TheCharlatan andrewtoth

Labels
UTXO Db and Indexes CI failed

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-11-21 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me