cloudwatchlogs: buffer resetTimerCh to prevent lost wakeup in force-flush timer#2166
Merged
Merged
Conversation
976f9fe to
950d65b
Compare
3872ff5 to
c690a7c
Compare
461771e to
70af2ad
Compare
127bb7f to
bcf8894
Compare
jefchien
reviewed
Jun 23, 2026
cf9fdeb to
1d86fc7
Compare
1d86fc7 to
84cad0b
Compare
2bf2aa3 to
5bdd682
Compare
okankoAMZ
reviewed
Jun 30, 2026
| logLines := logSink.Lines() | ||
| lastLine := logLines[len(logLines)-1] | ||
| expected := fmt.Sprintf("All %v retries to G/S failed for PutLogEvents, request dropped.", cnt.Load()-1) | ||
| require.True(t, strings.HasSuffix(lastLine, expected), fmt.Sprintf("Expecting error log to end with request dropped, but received '%s' in the log", logSink.String())) |
Contributor
There was a problem hiding this comment.
why are we no longer checking for last line? why do we check ever line instead?
Contributor
Author
There was a problem hiding this comment.
So we can find the drop message regardless of its position.
b908524 to
d2f703e
Compare
a9c3c14 to
816e057
Compare
4851433 to
ef0e56a
Compare
ef0e56a to
722ebc0
Compare
okankoAMZ
approved these changes
Jul 1, 2026
jefchien
approved these changes
Jul 2, 2026
This file contains hidden or 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
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.
Summary
A one-line concurrency fix that prevents the force-flush timer from permanently stalling on low-volume log streams.
Background
The CloudWatch Agent reads log files and publishes events to CloudWatch Logs. Each log stream has a force-flush timer (default 5 seconds) that sends accumulated events even when the batch is not full. This keeps delivery timely for low-volume streams.
The flush timer is one-shot: after it fires,
resetFlushTimer()must be called to start it again. If the reset is dropped, the timer stays dead and events only leave when the batch fills up, which for a quiet stream may take minutes or longer.Problem
resetFlushTimer()uses a non-blocking send:When
manageFlushTimeris blocked sending onflushCh(waiting for the main loop to accept a flush signal), it cannot receive fromresetTimerCh. With an unbuffered channel, the rearm hits thedefaultbranch and is silently dropped. After a size-based send, this can leave the force-flush timer permanently un-rearmed. A low-volume stream's trailing batch then sits unsent with no error or retry logged.Fix
A buffer of 1 parks the rearm signal so
manageFlushTimerpicks it up on its next loop iteration. The signal is idempotent (presence means "please reset"), so a buffer of 1 is sufficient.Tests
Three new tests exercise the fix:
TestResetTimerChIsBufferedresetTimerChhas buffer capacity 1; cap 0 reintroduces the bug.TestForceFlushTimerRearmsAfterHeldSendflushTimeouteven when a size-based send held the flush path.TestForceFlushTimer_SingleLowVolumeEventFlushes