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

SIM-BE Some transactions in the consensus queue get duplicated, potentially resulting in stuck processing #502

Open
denishacquin opened this issue Sep 17, 2024 · 5 comments

Comments

@denishacquin
Copy link
Contributor

denishacquin commented Sep 17, 2024

When submitting many transactions, it happens that they get stuck on pending.

When checking the backend logs, I see some errors:

~ ~ ~ ~ ~ TRANSACTION ALREADY IN PROCESS

And when printing the queued tx IDS and status in the backend, I see that many transactions apparently get added multiple times to the queue, not sure if this is expected:

Screenshot 2024-09-17 at 11 19 36 Screenshot 2024-09-17 at 11 16 38

Step to reproduce:

  • Spam write transactions to a contract from the simulator

Restarting the backend clears the clone-filled queue and gets the process unstuck.

@denishacquin
Copy link
Contributor Author

denishacquin commented Sep 17, 2024

Is this expected @cristiam86 @AgustinRamiroDiaz ?

Otherwise,see linked branch for a fix proposal.

The idea would be to avoid duplicates by checking if the tx ID exists in the queue first, and keeping track of processed txs.

@cristiam86
Copy link
Collaborator

Oh, yes, the transaction shouldn't be re-inserted in the queue. This could be fixed when we implement the nonce, it is the purpose of that field

@AgustinRamiroDiaz
Copy link
Contributor

AgustinRamiroDiaz commented Sep 17, 2024

This is highly related to #387. There was a patch done to print the TRANSACTION ALREADY IN PROCESS and not try to re-execute it.

The idea would be to avoid duplicates by checking if the tx ID exists in the queue first, and keeping track of processed txs.

This is an almost perfect solution, but there's a small race condition that could happen where:

  • the consumer pops the transaction from the queue to start consuming it
  • the loop checks for the transaction in all queues, and doesn't find it so it adds it

It would be a great improvement for 99% of the cases, so maybe it's a good idea to also add that patch

To make it 100% reliable I think we'll probably need to add some kind of synchronization between the 2 threads, like a lock or something

@cristiam86
Copy link
Collaborator

We should implement the same blockchain approach, if there is a transaction with that nonce, it get's rejected.

@denishacquin
Copy link
Contributor Author

let's wait for transaction hashes #479 and nonces #468 , which should help resolving this issue

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

When branches are created from issues, their pull requests are automatically linked.

3 participants