feat: only require reindexing when the index was on going to off #6203
+52
−46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Guix Automation has began to build this PR tagged as v21.1.0-devpr6203.c412d905. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v21.1.0-devpr6203.c412d905. The image should be on dockerhub soon. |
UdjinM6
reviewed
Aug 12, 2024
src/init.cpp
Outdated
| @@ -1917,19 +1917,19 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) | |||
| } | |||
|
|
|||
| // Check for changed -addressindex state | |||
| if (fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { | |||
| if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { | |||
| strLoadError = _("You need to rebuild the database using -reindex to change -addressindex"); | |||
There was a problem hiding this comment.
Makes sense I guess... let's also update the error message maybe?
Suggested change
| strLoadError = _("You need to rebuild the database using -reindex to change -addressindex"); | |
| strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex"); |
Same below. Will have to update tests too.
|
I think the title supposed to be "on going to off"? |
PastaPastaPasta
changed the title
PastaPastaPasta
force-pushed
the
only-need-reindex-one-way
branch
from
8ae011d
to
a58ebbe
Compare
|
Anything else needed here? |
|
PastaPastaPasta
force-pushed
the
only-need-reindex-one-way
branch
2 times, most recently
from
212a38e
to
884d659
Compare
Whoops! I swear I already did that locally, just forgot to push it I guess |
UdjinM6
requested changes
Aug 21, 2024
| @@ -42,12 +42,12 @@ def setup_network(self): | |||
| def run_test(self): | |||
| self.log.info("Test that settings can't be changed without -reindex...") | |||
| self.stop_node(1) | |||
| self.nodes[1].assert_start_raises_init_error(["-addressindex=0"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX) | |||
| self.nodes[1].assert_start_raises_init_error(["-addressindex=0"], "You need to rebuild the database using -reindex to enable -addressindex", match=ErrorMatch.PARTIAL_REGEX) | |||
There was a problem hiding this comment.
-addressindex=0 == disable
you don't need to reindex to disable it...
knst
reviewed
Sep 24, 2024
Comment on lines
44
to
45
| self.stop_node(1) | ||
| self.nodes[1].assert_start_raises_init_error(["-addressindex=0"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX) | ||
| self.start_node(1, ["-addressindex=0", "-reindex"]) | ||
| self.start_node(1, ["-addressindex=0"]) |
There was a problem hiding this comment.
-self.stop_node(1)
-self.start_node(1, ["-addressindex=0"])
+self.restart_node(1, "[-addressindex=0"])
knst
reviewed
Sep 24, 2024
UdjinM6
reviewed
Sep 24, 2024
PastaPastaPasta
force-pushed
the
only-need-reindex-one-way
branch
from
84ff7b7
to
492204d
Compare
|
Finally pulled in your commit 🙈 I forget about too many PRs |
UdjinM6
approved these changes
Oct 24, 2024
fix: init additional indexes in AppInitMain, adjust tests Update test/functional/feature_timestampindex.py don't need to reindex to disable in tests
PastaPastaPasta
force-pushed
the
only-need-reindex-one-way
branch
from
492204d
to
c96f9e0
Compare
|
no diff; squashed these 4 commits into 1 |
knst
approved these changes
Oct 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was done?
It does not seem reasonable that we need to reindex when turning indexes off. this logic was introduced in e078109
How Has This Been Tested?
Hasn't
Breaking Changes
None, basically
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.