Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reset chainman to allow reindex on failure #5405

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented May 31, 2023

Issue being fixed or feature implemented

current develop fails to reindex whenever there is an issue at node start (prints should not be overwriting a chainstate in debug.log)

What was done?

reset chainman to allow it re-initialize chainstate

How Has This Been Tested?

simulated an issue with

 if (!fReset) {
     strLoadError = _("DEBUG");
     break;
 }

Breaking Changes

should not be any but pls test

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@UdjinM6 UdjinM6 added this to the 19.2 milestone May 31, 2023
@@ -2022,6 +2022,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
LOCK(cs_main);
node.evodb.reset();
node.evodb = std::make_unique<CEvoDB>(nEvoDbCache, false, fReset || fReindexChainState);

chainman.Reset();
Copy link
Collaborator

@knst knst May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't reset pointer also to be sure that all destructors will be called too?

p.s. seems are Reset() already does everything properly and likely it would be like that in future, so, that's sufficient enough.... Just think like using RAII [when possible] is better because no hidden non-reinizialized data remains.

ogabrielides
ogabrielides previously approved these changes May 31, 2023
Copy link
Collaborator

@ogabrielides ogabrielides left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

This pull request has conflicts, please rebase.

current develop fails with `should not be overwriting a chainstate`
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK for squash merge

@UdjinM6 UdjinM6 merged commit 87863a6 into dashpay:develop Jun 6, 2023
@UdjinM6 UdjinM6 deleted the fix_reindex_on_failure branch June 6, 2023 16:31
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 11, 2023
current develop fails to reindex whenever there is an issue at node
start (prints `should not be overwriting a chainstate` in `debug.log`)

reset chainman to allow it re-initialize chainstate

simulated an issue with
```
 if (!fReset) {
     strLoadError = _("DEBUG");
     break;
 }
```

should not be any but pls test

- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants