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

Batch status check queries for pending transactions in txsub #3552

Closed
bartekn opened this issue Apr 20, 2021 · 2 comments · Fixed by #3563
Closed

Batch status check queries for pending transactions in txsub #3552

bartekn opened this issue Apr 20, 2021 · 2 comments · Fixed by #3563
Assignees
Labels
horizon performance issues aimed at improving performance

Comments

@bartekn
Copy link
Contributor

bartekn commented Apr 20, 2021

What version are you using?

2.1.0

What did you do?

@tomerweller was running into performance issues when submitting thousands of transactions to a standalone network. It looks like the problem is related to this code:

for _, hash := range sys.Pending.Pending(ctx) {
tx, err := txResultByHash(db, hash)
if err == nil {
logger.WithField("hash", hash).Debug("finishing open submission")
sys.Pending.Finish(ctx, hash, Result{Transaction: tx})
continue
}
if _, ok := err.(*FailedTransactionError); ok {
logger.WithField("hash", hash).Debug("finishing open submission")
sys.Pending.Finish(ctx, hash, Result{Transaction: tx, Err: err})
continue
}
if err != ErrNoResults {
logger.WithStack(err).Error(err)
}
}

With many pending transactions getting its' results, even when separate queries are fast, can cumulate to a very long updates time (few seconds time). The chart below shows the time distribution of tx status updates after an example ledger ingestion:
Screenshot 2021-04-20 at 00 21 07

What did you expect to see?

When checking the status of pending transaction we should send a single batch query instead of thousands queries asking for a single tx result. This should make this magnitudes faster.

When working on this please remember about two things:

  • Postgres param limit of around 65k. If a batch query needs to send more params it should be called multiple times.
  • The query used right now is searching for both normal txs and fee bump txs. The batch query should do the same thing.
@bartekn bartekn added performance issues aimed at improving performance horizon labels Apr 20, 2021
@ire-and-curses
Copy link
Member

How do we decide when we've accumulated enough status checks to send a single batch query? There's a tradeoff here between not waiting and batching for performance.

@bartekn
Copy link
Contributor Author

bartekn commented Apr 20, 2021

Currently we check it on every app tick which is every second, even when there are no pending txs.

@bartekn bartekn self-assigned this Apr 22, 2021
@bartekn bartekn modified the milestone: Horizon 2.5.0 Jun 14, 2021
bartekn added a commit that referenced this issue Jun 28, 2021
…ctions in txsub (#3563)

This commit modifies `txsub.Tick` to 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.

This was done due performance reasons. If the number of transactions in the
queue is large this slows down entire `Tick` function. Data in #3552 suggest
that previous method can take even 4s per `Tick` execution for around 1000
transactions in the queue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
horizon performance issues aimed at improving performance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants