Abort HA Realization Logic After Timeout #800
Draft
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
julianbrost
reviewed
Sep 9, 2024
A strange HA behavior was reported in [icingadb-787], resulting in both instances being active. The logs contained an entry of the previous active instance exiting the HA.realize() method successfully after 1m9s. This, however, should not be possible as the method's context is deadlined to a minute after the heartbeat was received. With introducing Settings.QuickContextExit for retry.WithBackoff in [icinga-go-library-69] and using it here, the function directly returns a context.DeadlineExceeded error the moment the context has expired. Doing so allows directly handing over, while the other instance can now take over due to the expired heartbeat in the database. As a related change, the HA.insertEnvironment() method was inlined into the retryable function to use the deadlined context. Otherwise, this might block afterwards, as it was used within HA.realize(), but without the passed context. In addition, the main loop select cases for hactx.Done() and ctx.Done() were unified, as hactx is a derived ctx. A closed ctx case may be lost as the hactx case could have been chosen. [icinga-go-library-69]: Icinga/icinga-go-library#69 [icingadb-787]: #787
oxzi
force-pushed
the
ha-desperate-timing-fixes
branch
from
46a9b12
to
a03246f
Compare
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.
A strange HA behavior was reported in #787, resulting in both instances being active.
The logs contained an entry of the previous active instance exiting the
HA.realize()method successfully after1m9s. This, however, should not be possible as the method's context is deadlined to a minute after the heartbeat was received.With introducing
Settings.QuickContextExitforretry.WithBackoffin Icinga/icinga-go-library#69 and using it here, the function directly returns acontext.DeadlineExceedederror the moment the context has expired. Doing so allows directly handing over, while the other instance can now take over due to the expired heartbeat in the database.As a related change, the
HA.insertEnvironment()method was inlined into the retryable function to use the deadlined context. Otherwise, this might block afterwards, as it was used withinHA.realize(), but without the passed context.In addition, the main loop select cases for
hactx.Done()andctx.Done()were unified, ashactxis a derivedctx. A closedctxcase may be lost as thehactxcase could have been chosen.This PR was marked as a draft as it requires Icinga/icinga-go-library#69 and a bumped icinga-go-library version.