Repository navigation
Conversation
When a log destination stopped, the queue's merge goroutine returned as soon as it saw stopCh. select picks at random among ready cases, so the events still buffered in the queue's channels were mostly left behind: they were never sent, and their done callbacks never ran. merge now forwards the events still buffered in both channels before it returns, so that the queue sends them before it exits. Part of aws#2398 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
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.
Description of the issue
Part of #2398. On its own this does not fix the case that issue reports; see the last paragraph.
When a log destination stops,
queue.StopclosesstopCh, andmergereturned as soon as it saw it. If events were still buffered ineventsCh, or in the non-blocking channel used for EMF,selectpicked at random between them andstopCh, somergeusually returned after forwarding a few of them. The rest were never sent, and their done callbacks never ran. For a tailed file, that means its offset did not advance, so the lines were read again after a restart, and lost if the instance was gone.Events wait there whenever the queue goroutine is busy in a
PutLogEventscall, which it makes itself unlessconcurrencyis set above 1. A destination stops when the agent shuts down in logs-only mode (CloudWatchLogs.Close), and when its last source stops (NotifySourceStopped).In #2398 the config has a
metricssection, so the agent runs the collector, and nothing callsCloudWatchLogs.Closeat exit. The tailer also keeps the newest line in its multi-line buffer and drops it on stop. Fixing those changes the shutdown order, so I'd like to agree on the approach in the issue first. This PR fixes a part that stands on its own and that such a fix needs: a shutdown that stops each destination after its sources publish their last lines would otherwise lose those lines here.Description of changes
On
stopCh,mergenow forwards the events still buffered ineventsCh, then those in the non-blocking channel, before it returns. The queue goroutine batches and sends them as before, and then makes its final send.cwDestadds events and stops the queue under its lock, so nothing is added afterStop, and an empty channel stays empty.Pusher.Stopstops the sender right after the queue, so each of these batches gets one attempt, as the final send already does. The drain adds to a stop at most what the channels hold: 100 events, or up to 20,000 EMF events.#2023 and #1975 also change
queue.go, but notmerge. With the stop-awareAddEventin #2023, an event added after the drain is dropped as it is today, and the drain still never blocks.License
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Tests
TestStopQueueWouldSendEventsWaitingInChannel, besideTestStopQueueWouldDoFinalSend, holds the queue goroutine in its firstPutLogEventscall, adds 50 events, stops the queue and then releases the call. It checks that all 51 events are sent, in order, and that each one's done callback ran. It runs once withAddEventand once withAddEventNonBlocking, and has no sleeps.mainat 7dbea90): 40 of 40 subtest runs fail, with 2 to 7 of the 51 events sent.-count=200, 400 of 400 subtest runs pass.The queue reproduction in #2398 (
TestReproStopDropsEventsWaitingInChannel) now sends all 51 events in 20 of 20 runs. Before, it dropped 46.All pass, and the
pusherpackage takes as long as onmain(128 s against 123 s).go vet,gofmt,goimports -local,impiandaddlicenseare clean on the changed files. golangci-lint v2.12.2, the Makefile's version, reports 0 issues on./plugins/outputs/cloudwatchlogs/.... Go 1.26.11, linux/arm64.CGO_ENABLED=1 go test -race -parallel 4(asmake test-data-raceruns it) passes on./plugins/outputs/cloudwatchlogs/internal/pusher/and./plugins/outputs/cloudwatchlogs/with no race reported, and so do the new test and the two existing stop tests with-race -count=50.Not run: the whole
make test, and the integration tests.A possible release note line:
* [Logs] Fix log events waiting to be sent being dropped when a log destination stopsRequirements
Before commiting your code, please do the following steps.
make fmtandmake fmt-shmake lintIntegration Tests
To run integration tests against this PR, add the
ready for testinglabel.🤖 Generated with Claude Code