-
Notifications
You must be signed in to change notification settings - Fork 499
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 horizon db reap command #2336
Conversation
When the db reap command is called, `ledger.CurrentState()` returns an uninitialized struct where the latest history ledger sequence is set to 0. Consequently, the reap system always concludes that there are no ledgers preceding the cutoff point. To fix this bug we need to call `app.UpdateLedgerState()` to ensure that ledger.CurrentState() is properly initialized. Another problem with the reap command is that it does not delete from all history tables (e.g. history_trades). This commit fixes this issue by calling `DeleteRangeAll()` which is used by the ingestion system to clear history tables prior to running a reingest range command.
two questions: I assume the fact that Should this PR be merged into master or should it remain in the 1.1.0 release? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tamirms good catch! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the fact that horizon db reap neglected to delete rows from the history_trades table was a mistake. Am I right or is that the actual intended behavior?
I think it was a bug, so 👍 for fixing it!
Should this PR be merged into master or should it remain in the 1.1.0 release?
I think we should only use the release branch for breaking changes and some new features we know require more testing before we can release.
Co-Authored-By: Bartek Nowotarski <bartek@nowotarski.info>
PR Checklist
PR Structure
otherwise).
services/friendbot
, orall
ordoc
if the changes are broad or impact manypackages.
Thoroughness
.md
files, etc... affected by this change). Take a look in the
docs
folder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Close #2307
When the db reap command is called,
ledger.CurrentState()
returns an uninitialized struct where the latest history ledger sequence is set to 0. Consequently, the reap system always concludes that there are no ledgers preceding the cutoff point. To fix this bug we need to callapp.UpdateLedgerState()
to ensure that ledger.CurrentState() is properly initialized.Another problem with the reap command is that it does not delete from all history tables (e.g. history_trades). This commit fixes this issue by calling
DeleteRangeAll()
which is used by the ingestion system to clear history tables prior to running a reingest range command.Known limitations
[N/A]