Improve failover robustness #8405
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
...ator/remote/src/main/java/tech/pegasys/teku/validator/remote/BeaconNodeReadinessManager.java
Outdated
Show resolved
Hide resolved
.../java/tech/pegasys/teku/validator/remote/eventsource/EventSourceBeaconChainEventAdapter.java
Show resolved
Hide resolved
|
Regarding the tests, It would be great to add a test that covers the |
| @@ -97,6 +100,45 @@ public void performsPrimaryReadinessCheckWhenFailoverNotReadyAndNoOtherFailovers | |||
| verify(beaconNodeReadinessManager).performPrimaryReadinessCheck(); | |||
| } | |||
|
|
|||
| @Test | |||
| public void dontJumpBetweenFailoversWhenFailoverIsReady() { | |||
There was a problem hiding this comment.
Maybe rename it to doNotSwitchToFailoverWhenCurrentBeaconNodeIsReady
mehdi-aouadi
approved these changes
Jun 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
Covered following cases:
switchToFailoverEventStreamIfAvailable()is not always fired when currently used api fails, it could be fired by connection error fallback for example and switch endpoint (not considering primary, when it could be the best) when not neededBeaconNodeReadinessManageris switched to primary as ready, sent event once and will not repeat it, but by some reason this event slipped inEventSourceBeaconChainEventAdapter(say callbackswitchToFailoverEventStreamIfAvailable()fired just after it). Repeated events guarantee that we will switch to primary in this case sooner or later and price is nothing.Wanted to add some tests, but it doesn't look it could be tested easily. Maybe you could provide some suggestion?
Fixed Issue(s)
Fixes #8180 (not 100% on this)
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog