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

fix(v7): prevent concurrent hermes operations on the same chain #1099

Merged
merged 2 commits into from
May 10, 2024

Conversation

fastfadingviolets
Copy link
Contributor

@fastfadingviolets fastfadingviolets commented Apr 25, 2024

Summary

Allowing concurrent ops can result in hermes create ... commands failing because there's sequence numbers being modified by simultaneous commands. This PR adds per-chain locks that must be acquired to e.g. create a connection or channel

@fastfadingviolets fastfadingviolets requested a review from a team as a code owner April 25, 2024 18:05
Copy link

vercel bot commented Apr 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
interchaintest-docs ⬜️ Ignored (Inspect) Visit Preview May 8, 2024 2:20pm

@fastfadingviolets fastfadingviolets force-pushed the fix/v7-hermes-lock branch 2 times, most recently from b102b5d to 6529729 Compare April 25, 2024 18:30
@Reecepbcups
Copy link
Member

After multiple re-runs I can not get the unit test or test-ibc-examples w/ hermes to pass here

@fastfadingviolets
Copy link
Contributor Author

o i probably broke something; i think i'll also have to bump a timeout

@fastfadingviolets
Copy link
Contributor Author

i think the tests should be good now, they're just currently failing with that transient block.db thing

@jtieri
Copy link
Member

jtieri commented Apr 29, 2024

i think the tests should be good now, they're just currently failing with that transient block.db thing

If it's not too much to ask could you possibly remove this line that is inside of the intermittently failing test case?

BlockDatabaseFile: interchaintest.DefaultBlockDatabaseFilepath(),

I thought we had gone through the example test cases and either removed all of these lines or commented them out so that readers could at least see that this was an option. It's not ideal to use in CI because it leads to these weird intermittent failures due to resource contention around the sqlite database file that is used in the blockdb

Allowing them can result in "hermes create ..." commands failing because
there's sequence numbers being modified by simultaneous commands.
@Reecepbcups Reecepbcups enabled auto-merge (squash) May 10, 2024 21:03
@Reecepbcups Reecepbcups merged commit 9a88474 into strangelove-ventures:v7 May 10, 2024
13 checks passed
@fastfadingviolets fastfadingviolets deleted the fix/v7-hermes-lock branch August 19, 2024 15:22
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

Successfully merging this pull request may close these issues.

3 participants