Skip to content

perf: share single lumberjack writer across loggers#20018

Draft
davdhacs wants to merge 1 commit intomasterfrom
davdhacs/pr1b-lumberjack
Draft

perf: share single lumberjack writer across loggers#20018
davdhacs wants to merge 1 commit intomasterfrom
davdhacs/pr1b-lumberjack

Conversation

@davdhacs
Copy link
Copy Markdown
Contributor

Description

Share a single lumberjack log rotation writer per file path instead of creating one per logger. Previously each of 100+ loggers created its own lumberjack writer for the same log file, resulting in ~30 rotation-watching goroutines.

Changes

  • Single shared lumberjack.Logger instance per output path
  • Rotation goroutines reduced from ~30 to 1

Measurements

  • Goroutines: -29 (rotation watchers)
  • Memory: ~1 MB savings

Depends on the zap sampling change (#19997, merged).

User-facing documentation

Testing and quality

  • the change is production ready
  • CI results are inspected

Automated testing

  • modified existing tests

How I validated my change

  • Verified log rotation behavior with shared writer
  • Deployed on live GKE test cluster

AI-assisted.

Each CreateLogger call created an independent lumberjack.Logger for
the same log file, spawning its own rotation goroutine. With ~30
loggers, that's 30 goroutines + 30 file handles to the same file.

Share a single writer per path via a map. This reduces log rotation
goroutines from 30 to 1 and eliminates potential corruption from
concurrent uncoordinated writes to the same file.

GC sweet spot experiment findings (included in commit message for context):
- 128Mi: GC thrashing (84 GC/min, 200m CPU)
- 160Mi: Sweet spot (2 GC/min, 4m CPU)
- 192Mi: Comfortable (0 GC/min, 3m CPU)
- Rule: set limit to 1.3-1.5x natural heap size

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@openshift-ci
Copy link
Copy Markdown

openshift-ci bot commented Apr 15, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@github-actions
Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 31f6f34. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-658-g31f6f34727

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

🚀 Build Images Ready

Images are ready for commit 31f6f34. To use with deploy scripts:

export MAIN_IMAGE_TAG=4.11.x-658-g31f6f34727

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.61%. Comparing base (4d2d701) to head (31f6f34).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/logging/logging.go 75.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master   #20018   +/-   ##
=======================================
  Coverage   49.61%   49.61%           
=======================================
  Files        2765     2765           
  Lines      208628   208635    +7     
=======================================
+ Hits       103509   103523   +14     
+ Misses      97462    97456    -6     
+ Partials     7657     7656    -1     
Flag Coverage Δ
go-unit-tests 49.61% <75.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant