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

Additional PendingTransactionInfo endpoint flag #3967

Open
barnjamin opened this issue May 10, 2022 · 1 comment
Open

Additional PendingTransactionInfo endpoint flag #3967

barnjamin opened this issue May 10, 2022 · 1 comment
Labels

Comments

@barnjamin
Copy link
Contributor

Problem

The function that looks up PendingTransactionInformation starts by checking the local cache for a given transaction ID. If it is found there it still iterates through up to 1000 blocks to look for the transaction just in case it's already been confirmed but not removed from the pending pool.

There was some resistance to change it in this #3836 because it may cause some compatibility issues or worse return incorrect or outdated information.

It seems to be wasted effort to look 1000 rounds in the past for a transaction that is still in pending since it should be removed after only a few rounds at the most.

Solution

Add an additional flag to the pending endpoint that returns the result in the pending pool if it's found here: https://github.com/algorand/go-algorand/blob/master/node/node.go#L641

Alternatively pass some max lookback parameter to prevent the caller from having to wait for 1k rounds in the past if the transaction was just submitted.

This will prevent us from looking back in historical blocks unnecessarily and make the endpoint much faster as well as cutting down on CPU for the most used endpoint for API providers.

One API provider provided the following as an example of the improvement when we return immediately after finding the transaction in the pending pool

image

@barnjamin barnjamin added the new-feature-request Feature request that needs triage label May 10, 2022
@jannotti
Copy link
Contributor

jannotti commented Sep 13, 2022

It seems to me that algod should be able to use the txtail to find out if the txid is in any of the blocks, without scanning over them. The txtail needs to retain txids in order to perform dupe detection.

Looking a bit more, it seems that roundCowState.checkDup() needs the lastValid of the txn it's trying to check. This leads me to believe that we throw away the info we want after a txn's lastvalid expires (when it can no longer cause a dup). So we would not always be able to look back 1000 rounds. If a txn has a last valid of 100, and it ran in block 90, we would not be able to look it up with this idea after block 101.

So perhaps this is, at best, an optimization, but we'd have to fall back to scan sometimes. (Also, since we store the IDs keyed by last valid, but don't have that when we want to do the lookup, we'd have to try them all. I still think it would be faster very often, because this would not mean parsing blocks, just doing several hash table lookups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants