heartbeat slightly more frequently on disruption #44715
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
fspmarshall
added
no-changelog
fspmarshall
force-pushed
the
fspmarshall/disruption-heartbeat-interval
branch
from
be3f9ca to
921cfa2
Compare
rosstimothy
reviewed
Jul 29, 2024
lib/utils/interval/interval_test.go
Outdated
Comment on lines
30
to
31
| // TestLastTick verifies that the LastTick method returns the last tick time as expected. Due to the | ||
| // flaky nature of time-based tests runs many cases and passes if >50% of them succeed. |
There was a problem hiding this comment.
Is it possible to use a fake clock instead?
Comment on lines
+134
to
+118
| // TestIntervalResetTo verifies the basic behavior of the interval ResetTo method. | ||
| // Since time based tests tend to be flaky, this test passes if it has a >50% success | ||
| // rate (i.e. >50% of ResetTo calls seemed to have changed the timer's behavior as expected). |
There was a problem hiding this comment.
Is it possible to use a fake clock instead?
fspmarshall
force-pushed
the
fspmarshall/disruption-heartbeat-interval
branch
from
921cfa2 to
2defa53
Compare
rosstimothy
reviewed
Aug 5, 2024
rosstimothy
approved these changes
Aug 7, 2024
ryanclark
approved these changes
Aug 7, 2024
public-teleport-github-review-bot
bot
removed the request for review
from fheinecke
|
@fspmarshall See the table below for backport results.
|
This was referenced Aug 12, 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.
This PR updates stream-based heartbeat logic to slightly increase the announce frequency for the first heartbeat after a disruption. It has been noticed during testing that if the auth server fails in a manner that doesn't result in the stream being immediately closed (deadlock, backend i/o timeout, etc), the agent may erroneously believe its most recent heartbeat was successful when it wasn't. This can lead to an agent's backend resource becoming very stale. If other factors (e.g. cpu constraints on agent, auth, or both) are at play, it may even allow the agent's heartbeat state in the backend to briefly expire, rendering it momentarily undialable. This change is intended to mitigate this issue without significantly increasing backend writes on a mass reconnection event.