services/horizon/txsub: Batch status check queries for pending transactions in txsub #3563
Conversation
| @@ -28,6 +28,25 @@ func (q *Q) TransactionByHash(ctx context.Context, dest interface{}, hash string | |||
| return q.Get(ctx, dest, union) | |||
| } | |||
|
|
|||
| // TransactionsByHashesSinceLedger fetches transactions from the `history_transactions` | |||
| // table which match the given hash since the given ledger sequence (for perf reasons). | |||
| func (q *Q) TransactionsByHashesSinceLedger(ctx context.Context, dest interface{}, hashes []string, sinceLedgerSeq uint32) error { | |||
There was a problem hiding this comment.
why is it necessary to add the ht.ledger_sequence >= sinceLedgerSeq condition? I thought the transaction hash is globally unique?
There was a problem hiding this comment.
Unfortunately the query without the ledger_sequence limit is extremely slow. I realized it after to.mer tested it and added a fix in e8701ab. Technically the 100 ledgers is too much because TransactionsByHashesSinceLedger is used only when checking the status of transactions in Tick so this could be 12 (one minute of ledgers).
| for _, hash := range pending { | ||
| tx, found := txMap[hash] | ||
| if !found { | ||
| continue |
There was a problem hiding this comment.
How come we can happily continue here?
There was a problem hiding this comment.
Couldn't it be that the transaction is in the DB but isn't found by TransactionsByHashesSinceLedger() due to the sinceLedgerSeq passed?
There was a problem hiding this comment.
If txMap doesn't contain the hash it means that tx with hash has not been included in the ledger yet. So we continue to the next pending transaction.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder 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
This commit modifies
txsub.Tickto check the status of transactions in the queue by sending a batch query instead of sending separate queries to get status of each transaction in the queue.Close #3552.
Why
This was done due performance reasons. If the number of transactions in the queue is large this slows down entire
Tickfunction. Data in #3552 suggest that previous method can take even 4s perTickexecution for around 1000 transactions in the queue.Known limitations
N/A