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

Reviews

See the guideline for information on the review process.

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:

• #26045 (rpc: Optimize serialization disk space of dumptxoutset by aureleoules)

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.

Thanks for the look @Sjors. I also realized that there were two outlier calls where we weren't asserting that Flush()/Sync() has succeeded, so I've fixed those.

re-utACK 937016b

From my testing of #15606 - without this PR - I get the impression that flushing isn't working, specifically that RAM is not freed up. It could just be confusion on my end from rebase hell, but I'd like to make sure we're not obscuring a bug by merging this. (I initially assumed the main assumeutxo PR already contained this PR because RAM didn't go down)

@Sjors how did you detect that RAM wasn't being freed? Did you verify that the flush did not happen based on an absence of logs?

Measures of memory can be deceptive; I remember bitcoinperf spuriously detecting an increase in memory usage because once claimed, the process' resident set size (RSS) doesn't necessarily go down upon free.

@jamesob using the system monitor desktop app in Ubuntu, and I believe also based on the cache size log messages. I'll double check this next time I run it.

Actually this is broken at the moment, since although we don't flush after snapshot load, we do flush (and not sync) in the MaybeRebalanceCaches() -> FlushStateToDisk() call right after activating the chainstate snapshot. Will close this and think about how to rework it.