-
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
services/horizon: Remove ingest failed transactions flag #2702
services/horizon: Remove ingest failed transactions flag #2702
Conversation
ff468ef
to
4a2b798
Compare
4a2b798
to
34c1541
Compare
b31dc86
to
0aa3e6a
Compare
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.
LGTM! I wonder if we should makes sure that users who use Horizon DB directly (which is not recommended!) know what is the result of this change. Added a suggestion..
Co-authored-by: Bartek Nowotarski <bartek@nowotarski.info>
} | ||
|
||
var _ txsub.ResultProvider = &DB{} | ||
|
||
// ResultByHash implements txsub.ResultProvider | ||
func (rp *DB) ResultByHash(ctx context.Context, hash string) txsub.Result { | ||
historyLatest := ledger.CurrentState().HistoryLatest | ||
|
||
// query history database | ||
var hr history.Transaction | ||
err := rp.History.TransactionByHash(&hr, hash) |
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 do we allow tx submission if Horizon is catching up with stellar-core? could this potentially cause a hash not to be found (since it hasn't been ingested)?
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.
that's a good point. I think this issue you've spotted was already there before this PR so we can try to tackle it in another PR
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
Remove ingest failed transactions flag so that Horizon will always ingest failed transactions
Why
Having failed transactions in the Horizon DB is useful because it allows us to remove the last remaining dependency on the stellar core DB in the txsub module.
Known limitations
There might be some operators who prefer to save space by not ingesting failed transactions in the Horizon DB. However, there is a reap system in Horizon which allows you to remove older rows in the history tables. Using horizon reap should satisfy the same need.