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: avoid race condition when building chain processors #288

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

byte-bandit
Copy link
Collaborator

@byte-bandit byte-bandit commented Sep 25, 2023

Background

I'm surprised this hasn't flared up before. Pigeons will usually "build processors" before every operation, meaning they will ask Paloma for the latest chain configuration and compare their local setup. In case there's a difference, they will rebuild their local setup to match what's asked for by Paloma.

The code for this was not written with concurrency in mind, but is part of most of our background tasks which all run concurrently. Therefore, updating the internal state would interfere with each other, leading to contradicting results, a ton of update calls, invalid and incomplete data in the valsets and more.

This change adds a read/write mutual exclusion lock around the code in question, ensuring that reads stay performant, but don't interfere with updating the internal state any longer.

Testing completed

  • test coverage exists or has been added/updated
  • tested in a private testnet

Breaking changes

  • I have checked my code for breaking changes

@byte-bandit byte-bandit requested a review from taariq September 25, 2023 21:22
@byte-bandit byte-bandit enabled auto-merge (squash) September 25, 2023 21:33
Copy link
Contributor

@taariq taariq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reflect: we now have a new set of mutex locks to prevent race conditions on getting chain infos. Yes, surprised this was not an issue before. Any tests needed here?

@byte-bandit byte-bandit merged commit d3babf4 into palomachain:master Sep 25, 2023
@taariq taariq deleted the clohr/chain-info-log branch September 25, 2023 22:58
@byte-bandit
Copy link
Collaborator Author

To reflect: we now have a new set of mutex locks to prevent race conditions on getting chain infos. Yes, surprised this was not an issue before. Any tests needed here?

@taariq There's tests in place that verify the func works as expected.

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.

2 participants