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

CircuitBreaker transition into next states only when previous state aligns with what has been read #1064

Closed
wants to merge 4 commits into from

Conversation

danielmarbach
Copy link
Contributor

@danielmarbach danielmarbach commented Oct 1, 2024

This should make sure the following outcomes happen

Thread 1 calls Failure():

  • Reads previousState as Disarmed.
  • Attempts to change state from Disarmed to Armed using Interlocked.CompareExchange.
  • If successful, it proceeds to invoke armedAction() and starts the timer.

Thread 2 calls Success():

  • Reads previousState as Armed (if Thread 1 has already changed it).
  • Attempts to change state from Armed to Disarmed using Interlocked.CompareExchange.
  • If successful, it proceeds to invoke disarmedAction() and stops the timer.

Possible Outcomes:

  • If Thread 1's state change occurs before Thread 2 reads the state:
    • Thread 2 sees previousState as Armed and successfully disarms the circuit breaker.
  • If Thread 2 reads the state before Thread 1 changes it:
    • Thread 2 sees previousState as Disarmed and returns early without doing anything.
  • Regardless of the timing, actions are only performed when the state transitions are successful, preventing race conditions.

Additionally, this PR synchronizes access to updating the processor processing capacity because armed and disarmed actions can potentially be executed concurrently. Setting concurrency and prefetch count is thread safe in the SDK, but we want to make sure the processor state updates are done consistently.

@danielmarbach danielmarbach changed the title Doing extra checks for more safety CircuitBreaker transition into next states only when previous state aligns with what has been read Oct 1, 2024
@danielmarbach
Copy link
Contributor Author

Closed in favor of #1066

@danielmarbach danielmarbach deleted the more-sanity-checks branch October 1, 2024 22:16
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.

1 participant