Skip to content

Fix kmsg watcher restart and shutdown issues#1311

Merged
kubernetes-prow[bot] merged 3 commits into
kubernetes:masterfrom
hakman:kmsg-watcher-fixes
Jul 11, 2026
Merged

Fix kmsg watcher restart and shutdown issues#1311
kubernetes-prow[bot] merged 3 commits into
kubernetes:masterfrom
hakman:kmsg-watcher-fixes

Conversation

@hakman

@hakman hakman commented Jul 11, 2026

Copy link
Copy Markdown
Member

Review of the recent kmsg restart changes (#1192, #1245, #1259) surfaced three issues, one commit each:

  • Rate-limit restarts: a parser whose channel closes right after a successful restart caused a hot restart loop with no delay (measured 37k restarts in 200ms).
  • Stop() deadlock: with logCh full and the consumer already stopping, watchLoop blocked on a plain send and Stop() hung.
  • Double close: when Stop() interrupts a restart attempt, the deferred cleanup closed the already-closed old parser, logging a spurious error.

Each fix comes with a regression test; the package passes go test -race.

hakman added 3 commits July 11, 2026 10:27
If the restarted parser's channel closes again right away (reads keep
failing on the reopened /dev/kmsg), the watcher restarts in a tight
loop with no delay, spinning a CPU core and flooding the logs
(measured 37k restarts in 200ms). Delay the first attempt when the
previous restart was less than retryDelay ago.
The log monitor stops draining logCh before calling watcher.Stop(), so
with a full channel (e.g. a kmsg burst at shutdown) watchLoop blocked
on the send forever, never called tomb.Done(), and Stop() hung.
Select on tomb.Stopping() alongside the send.
…nterrupts a restart

The restart path closes the failed parser before retrying. If stopping
is signaled during the retry wait, watchLoop's deferred cleanup closed
the same parser again, logging a spurious 'file already closed' error
at shutdown. Clear the reference after closing and nil-check the defer.
@kubernetes-prow kubernetes-prow Bot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 11, 2026
@kubernetes-prow kubernetes-prow Bot requested review from mrunalp and yujuhong July 11, 2026 07:29
@kubernetes-prow kubernetes-prow Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 11, 2026
@hakman

hakman commented Jul 11, 2026

Copy link
Copy Markdown
Member Author

/cc @ameukam @upodroid

@kubernetes-prow kubernetes-prow Bot requested review from ameukam and upodroid July 11, 2026 07:50
@ameukam

ameukam commented Jul 11, 2026

Copy link
Copy Markdown
Member

/lgtm

@kubernetes-prow kubernetes-prow Bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2026
@kubernetes-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ameukam, hakman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot merged commit 1a2203f into kubernetes:master Jul 11, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants