test: fix feature_init.py intermittencies

furszy commented at 2:41 pm on June 30, 2025: member

Aims to solve #32600. Found it while working on #26966 (this was really annoying there).

This ensures the node is index-synced before perturbing files. If the index sync gets interrupted before it starts, the database could be empty, making any following perturbation ineffective (which explains why the node does not abort during startup in the #32600 logs).

Also, the first commit avoids initializing components not under test. This reduces log flooding, which helped in understanding the issue.

Patch to reproduce the issue on master using feature_init.py (this simulates a node shutting down before the index starts syncing):

 0diff --git a/src/index/base.cpp b/src/index/base.cpp
 1--- a/src/index/base.cpp	(revision 1e03052c3fefb188f047e72548f2c6b0cc019e50)
 2+++ b/src/index/base.cpp	(date 1751293306725)
 3@@ -185,6 +185,7 @@
 4 void BaseIndex::Sync()
 5 {
 6     const CBlockIndex* pindex = m_best_block_index.load();
 7+    m_interrupt();
 8     if (!m_synced) {
 9         std::chrono::steady_clock::time_point last_log_time{0s};
10         std::chrono::steady_clock::time_point last_locator_write_time{0s};

test: feature_init, only init what's needed per perturbation/deletion round

Avoids initializing and syncing components not under test.
This not only speeds up execution a bit but also helps isolate
and debug issues more easily, as logs aren't flooded with
unrelated details.

dcd3343dda

DrahtBot renamed this:
~~test: fix feature_init.py intermittencies~~
test: fix feature_init.py intermittencies
on Jun 30, 2025

DrahtBot added the label Tests on Jun 30, 2025

DrahtBot commented at 2:41 pm on June 30, 2025: 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/32835.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Approach ACK	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:

#32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by TheCharlatan)
#30469 (index: Fix coinstats overflow 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.

TheCharlatan commented at 2:58 pm on June 30, 2025: contributor

Approach ACK

in test/functional/feature_init.py:106 in 1e03052c3f outdated

 99@@ -100,39 +100,90 @@ def check_clean_start():
100         check_clean_start()
101         self.stop_node(0)
102 
103+        # Prior to deleting/perturbing index files, ensure indexes are synchronized (i.e., data exists to modify)
104+        self.start_node(0, extra_args=args)
105+        tip_height = node.getblockcount()
106+        self.wait_until(lambda: all(i["synced"] and i["best_block_height"] == tip_height for i in node.getindexinfo().values()))
107+        self.stop_node(0)

maflcko commented at 3:07 pm on June 30, 2025:

any reason to start the node again? Can’t this wait_until be done in check_clean_start?

furszy commented at 3:30 pm on June 30, 2025:

any reason to start the node again? Can’t this wait_until be done in check_clean_start?

Just because check_clean_start() is also called inside the deletion rounds and didn’t want to start all indexes there on each cycle round. But that seems easily solvable.. one minute..

furszy commented at 3:37 pm on June 30, 2025:

Done.

furszy force-pushed on Jun 30, 2025

in test/functional/feature_init.py:104 in 2a9ae6f162 outdated

 99@@ -97,42 +100,88 @@ def check_clean_start():
100             self.log.debug("Terminating node after terminate line was found")
101             sigterm_node()
102 
103-        check_clean_start()
104+        # Prior to deleting/perturbing index files, ensure indexes are synchronized (i.e., data exists to modify)
105+        check_clean_start(args, lambda n: self.wait_until(lambda h=n.getblockcount(): all(i["synced"] and i["best_block_height"] == h for i in n.getindexinfo().values())))

maflcko commented at 3:41 pm on June 30, 2025:

this fails lint and getindexinfo is already a no-op when no args are provided, so no need to conditionally run it? could just inline it?

furszy commented at 3:53 pm on June 30, 2025:

this fails lint and getindexinfo is already a no-op when no args are provided, so no need to conditionally run it? could just inline it?

Saw it late, pushed.

furszy force-pushed on Jun 30, 2025

DrahtBot added the label CI failed on Jun 30, 2025

DrahtBot commented at 3:44 pm on June 30, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45063074304 LLM reason (✨ experimental): Lint check failed due to a Python code style error identified by ruff.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly 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.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

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

test: feature_init, ensure indexes are synced prior to perturbing files e36e0b1aeb

furszy force-pushed on Jun 30, 2025

DrahtBot removed the label CI failed on Jun 30, 2025

test: fix feature_init.py intermittencies #32835

Code Coverage & Benchmarks

Reviews

Conflicts