Add reindex=auto flag to automatically reindex corrupt data

AaronDewes commented at 5:35 pm on December 9, 2022: none

This allows the reindex flag to be set to auto, which automatically starts a reindex if the chain state or block index are corrupt. This can be especially useful for Raspberry Pi based full nodes, which often experience power outages or similar issues which can corrupt data.

It allows full node operators to make Bitcoin Core reindex automatically, without having to worry about removing the reindex flag again. (Which isn’t much effort, but can be annoying to forget).

In case this error message is printed, pretty much nothing else except a reindex can be done, so adding this feature would make user experience slightly better.

Rebased version of #22072.

Add reindex=auto flag to automatically reindex corrupt data

This allows the reindex flag to be set to auto, which automatically starts a reindex if the chain state or block index are corrupt.
This can be especially useful for Raspberry Pi based full nodes, which often experience power outages or similar issues which can corrupt data.
It allows full node operators to make Bitcoin Core reindex automatically, without having to worry about removing the reindex flag again. (Which isn't much effort, but can be annoying to forget)
In case this error message is printed, pretty much nothing else except a reindex can be done, so adding this feature would make user experience slightly better.

6d7052863a

DrahtBot commented at 5:35 pm on December 9, 2022: contributor

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

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25977 (refactor: Replace std::optional<bilingual_str> with util::Result by ryanofsky)
#25722 (refactor: Use util::Result class for wallet loading by ryanofsky)
#25665 (refactor: Add util::Result failure values, multiple error and warning messages by ryanofsky)
#25193 (indexes: Read the locator’s top block during init, allow interaction with reindex-chainstate by mzumsande)

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.

sipa commented at 8:08 pm on December 9, 2022: member

Not an argument against this PR, but I feel that if you’re running on a system where the chainstate is continuously corrupted, you should investigate what’s causing that and/or switch to non-broken hardware.

AaronDewes commented at 10:03 am on December 10, 2022: none

Not an argument against this PR, but I feel that if you’re running on a system where the chainstate is continuously corrupted, you should investigate what’s causing that and/or switch to non-broken hardware.

This was also mentioned in the original PR, but in my opinion, it helps especially for the Raspberry Pi users, who often don’t have protections against power outages (Using software like runcitadel or Umbrel). So while checking the hardware should be necessary, this could help improve the user experience for users on such projects or users who are fine with the risk.

fanquake commented at 10:22 am on December 10, 2022: member

Concept ~0

you should investigate what’s causing that and/or switch to non-broken hardware.

I agree.

(Using software like runcitadel or Umbrel).

Sounds like an option that might just be (ab)used by node in a box producers, (i.e on by default) to stop people complaining to them about hardware issues.

protections against power outages or users who are fine with the risk.

Users who are fine with these risks can also introduce their own solutions to this problem (i.e some system service/watchdog that doesn’t need to be a part of Bitcoin Core). There are also non-software solutions to freuqent/sudden power outages, i.e a UPS.

I don’t think allowing users to blindly ignore known/recurring problems and continue on, is something we should be supporting.

AaronDewes commented at 10:35 am on December 10, 2022: none

i.e some system service/watchdog that doesn’t need to be a part of Bitcoin Core

Yes, but implementing something like this would probably be a lot more complicated than just having this feature in Bitcoin Core (It would probably work by reading log messages, but if these change, the script would need to be changed, also, such a script would also need to check if the database still needs a reindex when Core restarts).

I don’t think allowing users to blindly ignore known/recurring problems and continue on, is something we should be supporting.

For some, it may be a one-time issue. And in my opinion, it could benefit node-in-a-box providers a lot. I wouldn’t call that “abusing a feature”. It definitely won’t help with users having HW issues.

As a former member of the Umbrel team, I know that on such software, HW issues caused a lot of other problems. A reindex was in almost every case only needed because of a power outage, which happens sometimes, but not often for most users.

maflcko commented at 10:37 am on December 10, 2022: member

I think it would be more useful to refuse a reindex on two subsequent starts, see #22072 (comment) . It is common for users to set the flag, then start, wait for the reindex to finish and then forget about the flag. On the next restart they will see that their whole sync progress is thrown away without warning, forcing them to wait hours/days/weeks (depending on their hardware) for no reason.

AaronDewes commented at 10:39 am on December 10, 2022: none

I also think it would be more useful to refuse a reindex on two subsequent starts

Where would you store such data? In a separate lock file?

maflcko commented at 10:41 am on December 10, 2022: member

It might be sufficient to just shut down right after the reindex has started

AaronDewes commented at 10:45 am on December 10, 2022: none

It might be sufficient to just shut down right after the reindex has started

Do you mean after it finished? I was talking about how Core should know it reindexed on the previous start. I don’t understand how a shutdown could help?

maflcko commented at 10:54 am on December 10, 2022: member

It just seems better UX if the user can do both edits (adding and removing the flag) in the shortest time possible.

Previous workflow:

User adds reindex -> reindex for 2 weeks -> … -> Restart node -> Another accidental reindex for 2 weeks -> …

Proposed workflow:

User adds reindex -> Program shuts down -> User removes reindex -> Program continues reindex (just once, instead of N times)

achow101 commented at 3:42 pm on April 25, 2023: member

The feature request didn’t seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.

Closing due to lack of interest. Pull requests with improvements are always welcome.

achow101 closed this on Apr 25, 2023

bitcoin locked this on Apr 24, 2024

Add reindex=auto flag to automatically reindex corrupt data #26674

Reviews

Conflicts