contrib: Support re-writing linearize-data dumps #31888

midnightmagic commented at 12:32 AM on February 17, 2025: contributor

For tools using the linearization scripts to continuously create a data dump it is undesirable to truncate the dumped file every time it is written. Doing so makes it harder to be directly consumed by an external tool. To fix this, change the file open mode to not truncate the entire file. After this change dumps are byte-for-byte overwritten. Some trailing data at the end of the file might be left over from a previous run in case of a reorg. To guard against this, clip off any remaining data at the end of the file once the dump is completed.

(small patch being passed around which I did not write but thought would be helpful to the community)

DrahtBot commented at 12:32 AM on February 17, 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/31888.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	sedited, maflcko

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

DrahtBot commented at 1:22 AM on February 17, 2025: contributor

🚧 At least one of the CI tasks failed. <sub>Debug: https://github.com/bitcoin/bitcoin/runs/37309076255</sub>

<details><summary>Hints</summary>

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.

</details>

DrahtBot added the label CI failed on Feb 17, 2025

laanwj added the label Scripts and tools on Feb 17, 2025

laanwj commented at 11:42 AM on February 17, 2025: member

Lint is failing on the formatting of the commit message:

[10:12:47.126] The subject line of commit hash 0cf4a6245d2bc8d57f91dd2b4f5bd5a0eed18ed4 is followed by a non-empty line. Subject lines should always be followed by a blank line.
[10:12:47.126] ^^^
[10:12:47.126] 
[10:12:47.126] ^---- ⚠️ Failure generated from lint check 'commit_msg' (Check that commit messages have a new line before the body or no body at all.)!

It also looks like the changes somehow break the feature_loadblock.py test, which uses the linearize script

midnightmagic force-pushed on Feb 19, 2025

maflcko commented at 9:59 AM on February 19, 2025: member

It would be good to include a test, or modify the existing one. Otherwise, it will be harder to test and see what behavior actually changed and future changes could break the fix again.

correct the linearize-data script (minor)

this clips off unwanted final data in event of reorg and fixes the file open
mode to prevent the truncation of files reopened

small patch being passed around which I did not write but thought would be
helpful to the community at large

e9dc16194b

midnightmagic force-pushed on Feb 19, 2025

DrahtBot removed the label CI failed on Feb 19, 2025

theStack commented at 2:12 PM on March 12, 2025: contributor

Can you give more context on what problem this PR is trying to solve? Looking at the description and the diff I can't follow, e.g. it's unclear to me what an "incorrect open mode" is.

midnightmagic commented at 5:01 AM on April 23, 2025: contributor

The changes function to make linearize "less-perturb" on-disk data. Altogether, in the event of a block reorganization, the reorg'd blocks are sometimes shorter than prior blocks' data in individual output files when split on e.g. size. When the writer potentially moves on to the next blk data file therefore, the rest of the block data that pre-existed the reorganization might need to be truncated off the end of that file before moving on to the next one.

For the "wb" Python open mode, all pre-existing block files as it writes out blk* files are destroyed and re-created by an open with "wb", which in python will auto-truncate any pre-existing file to 0 bytes—for many large files, something pulling data from the source will always see rapidly-changing source dataset all the time.

With the change, the attempt to verify that the file doesn't pre-exist and then re-write in-place from 0 will be much less likely to break anything trying to read the files, most of them will effectively never change, and instead we just get the (much less likely) chance of a reorg write racing a potential midstream read, plus the much less-likely truncate-eliminating-a-bit-of-end-file data.

It's not really perfect when considered in conjunction with other tools, but it's much better than the current behaviour. Probably some form of atomic file modification would be "better" in the sense of guaranteeing on-disk state consistency, or, perhaps a trigger to external tools to identify "current" consistent state, but in the meanwhile I can't really see this change hurting because of how shallow reorgs usually are and the inordinate size of the dataset we have to manipulate.

The use case is for people who repeatedly re-run linearize and make use of the results on an ongoing basis.

bitcoin31888 commented at 2:57 AM on May 19, 2025: none

Consider the following bash script

while true;do ./linearize-hashes.py linearize.cfg > hashlist.txt; ./linearize-data.py linearize.cfg; sleep 120; done

This will continuously write out the bootstrap files, without this patch the files will continuously be truncated to zero length and then re-written, that behavior is undesirable.

achow101 requested review from Eunovo on Oct 22, 2025

sedited commented at 7:54 AM on March 4, 2026: contributor

There's not been a response from the author to a review comment here, in quite some time. There has also been no further review. Maybe this should be closed and marked as up for grabs?

maflcko commented at 8:50 AM on March 4, 2026: member

My reading from #31888 (comment) seems to give an example of why this pull is needed. However, I haven't looked closely and reviewed/tested this myself

sedited commented at 9:11 AM on March 4, 2026: contributor

My reading from #31888 (comment) seems to give an example of why this pull is needed. However, I haven't looked closely and reviewed/tested this myself

Huh, yes, I did read that the wrong way around. Will take a look myself then :)

sedited commented at 12:03 PM on March 4, 2026: contributor

I'm not sure about this patch being framed as a correction of a wrong assumption. As described in a later comment, this adds support for writing to the same set of files again. I can see why that might be useful, but the description both for the PR and the commit should clarify this. Would also be good to capture this behaviour in the README.

midnightmagic commented at 7:21 PM on March 4, 2026: contributor

The use case described in that comment is precisely demonstrating a logical outcome of the regressive, drive-destroying behaviour for anyone who wants to use this script more than once, so, I'd thought it was a good example of what the goal of the diff is and didn't need comment. Does it need further comment? :-)

sedited commented at 9:36 AM on March 5, 2026: contributor

The use case described in that comment is precisely demonstrating a logical outcome of the regressive, drive-destroying behaviour for anyone who wants to use this script more than once, so, I'd thought it was a good example of what the goal of the diff is and didn't need comment. Does it need further comment?

To be clear, I agree it is a desirable thing to patch and the examples in the discussion here clarify that. I'm just saying that this should be clearly described in the pull request description and the commit message.

midnightmagic commented at 8:32 PM on March 5, 2026: contributor

Sure, I can flesh it out more if you like and make it more explicit.

sedited commented at 1:13 PM on March 9, 2026: contributor

Sure, I can flesh it out more if you like and make it more explicit.

Yes, that's all that's required to make progress here.

sedited commented at 8:04 PM on April 18, 2026: contributor

@midnightmagic , would be good to get this in finally, but I'd still like the description to say explicitly that this change targets behaviour around re-writing the files.

sedited renamed this:
~~correct wrong assumptions in the contrib linearize data script~~
contrib: Support running linearize-data multiple times
on May 22, 2026

sedited renamed this:
~~contrib: Support running linearize-data multiple times~~
contrib: Support re-generating linearize-data dumps
on May 22, 2026

sedited renamed this:
~~contrib: Support re-generating linearize-data dumps~~
contrib: Support re-writing linearize-data dumps
on Jun 19, 2026

sedited approved

sedited commented at 1:15 PM on June 19, 2026: contributor

ACK e9dc16194b47921ea6c5453e71c5c97b9232ba8c

Some of the reasons stated here don't make much sense to me, i.e. this doesn't seem to materially change disk wear. But I don't think this hurts existing users and if it can make things easier for the few upstream users this script has, that seems like a good thing.

sedited commented at 4:08 PM on September 21, 2026: contributor

This is a fairly straight forward change that seems to make things a bit easier for some external tools using these dumps. However, it's not getting review. Going to give this another round, and if it still isn't getting enough attention, will close it again due to lack of interest. Ping @theStack @maflcko @Eunovo.

maflcko commented at 4:57 PM on September 21, 2026: member

lgtm ACK e9dc16194b47921ea6c5453e71c5c97b9232ba8c

sedited merged this on Sep 21, 2026

sedited closed this on Sep 21, 2026