Fix and improve txsub package
#695
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes an issue that caused sending `Timeout` responses to client submitting transactions. The problem was present in `TransactionByHashAfterLedger` function: it was checking the result of the transaction in History DB and, when it was not found, in Core DB but with ledger sequence after the latest ingested ledger. Given the fact that Horizon ingests only successful transactions it was impossible to find results for some of the failed transactions (especially those failed at the consensus level that require more time to process). `SubmissionTimeout` was changed to 30 seconds because HTTP clients in SDKs usually timeout in 60 seconds. We want `SubmissionTimeout` to be lower than that to make sure that they read the response before the client timeout and 30 seconds is 6 ledgers (with avg. close time = 5 sec), enough for stellar-core to drop the transaction if not added to the ledger and ask client to try again by sending a `Timeout` response. It also contains improved logging in `txsub` package.
|
this seems fine as a quick fix but we should make sure to fix this when we ingest failed transactions: there is an inherent race condition with that approach as it's possible that core deletes transactions for old ledgers as soon as Horizon completes its ingestion |
Thanks for reminding about this. Testing it at horizon-testnet.stellar.org and results are great so far!
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes an issue that caused sending
Timeoutresponses to client submitting transactions. The problem was present inTransactionByHashAfterLedgerfunction: it was checking the result of the transaction in History DB and, when it was not found, in Core DB but with ledger sequence after the latest ingested ledger. Given the fact that Horizon ingests only successful transactions it was impossible to find results for some of the failed transactions (especially those failed at the consensus level that require more time to process).SubmissionTimeoutwas changed to 30 seconds because HTTP clients in SDKs usually timeout in 60 seconds. We wantSubmissionTimeoutto be lower than that to make sure that they read the response before the client timeout and 30 seconds is 6 ledgers (with avg. close time = 5 sec), enough for stellar-core to drop the transaction if not added to the ledger and ask client to try again by sending aTimeoutresponse.It also contains improved logging in
txsubpackage.