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

services/horizon: remove internal transaction submission queue #4996

Closed
mollykarcher opened this issue Jul 28, 2023 · 2 comments · Fixed by #5039
Closed

services/horizon: remove internal transaction submission queue #4996

mollykarcher opened this issue Jul 28, 2023 · 2 comments · Fixed by #5039

Comments

@mollykarcher
Copy link
Contributor

mollykarcher commented Jul 28, 2023

What problem does your feature solve?

Horizon manages an internal submission queue. This intent of this is to protect/isolate clients from potentially receiving error messages on transaction submission when multiple transactions are submitted very quickly in succession. If transactions (from the same source account) are submitted with sequence numbers out-of-order, then this submission queue will hold those transactions in-memory so that it can re-order them in line with their sequence numbers, allowing all of them to succeed when they are actually submitted to core. This behavior has limited benefit:

  • It is unclear which end-users or clients that this use case supports
  • The queue is in-memory, so the benefit it provides is not realized on load-balanced or horizontally scaled instances (ex. horizon.stellar.rg)
  • This behavior is confusing to end-users, and obfuscates underlying network behavior from the end user (ie. you must submit sequential sequence numbers)
  • Partners have been surprised by this behavior, causing outages (ie. Circle's bulk transaction submission incident)
  • Submitting out-of-order increases the probability of txsub requests 504-ing, in which case clients need to poll Horizon to validate their transaction was submitted

Recently, core has released a change that prevents the submission of multiple transactions in a ledger for a given account. With this change, the Horizon submission queue is even less likely to provide any benefit to end-users, and if anything cause more confusion. Transactions would be queued up in-memory, only to have only one of them be able to be submitted when they were finally submitted to core.

What would you like to see?

We should remove the transaction submission queue entirely, and simply pass-through transaction submission requests directly to core. This has the potential to result in more tx_bad_seq errors for clients that were relying on this behavior. However, it drastically simplifies the interface, makes the underlying network behavior clearer, and reduces user confusion that could have been exacerbated by the recent core update.

We'll need to be vigilant of monitoring any partner issues relating to this. Any partners that were relying on this behavior should be encouraged to migrate to a channel accounts setup, and we may need to lean-in in helping them do that.

What alternatives are there?

Not alternatives per se, but potential add-ons/scope increases we can consider:

  • This issue further simplifies the developer experience of transaction submission. With the submission queue fully removed, transaction submission should always be able to respond immediately with the knowledge that the transaction was at least accepted/validated by core. Note that there is still the possibility that it could not be included in a ledger, so clients would still need to poll Horizon to be sure.
  • Creating a library/helper/something that enables partners to quickly drop-in a channel account implementation if/when they do have issues with this. Note that anchor team had previously prioritized something similar, but the status of that appears stalled and it's future priority is unclear.
@sreuland
Copy link
Contributor

sreuland commented Aug 4, 2023

when the submission queue is removed, will trigger removal of the history_transactions_filtered_tmp table and the associated filtered out processor, which was added after filtering was added to ensure the submission queue results processor still could access tx's that were dropped by filtering and not added to history_transactions, refer to #4418 which added this layer, since submission queue will be removed, this additional layer of filtered out processor and tmp db table for submission queue can be removed.

@sydneynotthecity
Copy link

We should double-check that we don't document the queue behavior anywhere so that we do not mislead people

@sydneynotthecity sydneynotthecity moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Aug 15, 2023
@sydneynotthecity sydneynotthecity moved this from Current Sprint to Next Sprint Proposal in Platform Scrum Aug 15, 2023
@sydneynotthecity sydneynotthecity moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Aug 15, 2023
@aditya1702 aditya1702 moved this from Current Sprint to In Progress in Platform Scrum Aug 29, 2023
@aditya1702 aditya1702 moved this from In Progress to Needs Review in Platform Scrum Sep 12, 2023
@github-project-automation github-project-automation bot moved this from Needs Review to Done in Platform Scrum Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants