util/log: start garbage collection routines for old log files#68553
util/log: start garbage collection routines for old log files#68553craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
knz
left a comment
There was a problem hiding this comment.
Yep, this change looks right.
As we discussed I encourage you to enhance the existing file GC test to protect against regressions.
the release note could use more words though. Something like this:
"""
Release note (bug fix): File logs now respect the max-group-size configuration parameter again. This parameter had incorrectly become ignored in v21.1, resulting in arbitrarily large log directories.
"""
(release notes for bug fixes should mention when the bug was introduced and what were the symptoms of it occurring)
Reviewable status:
complete! 0 of 0 LGTMs obtained
a751077 to
8547db3
Compare
knz
left a comment
There was a problem hiding this comment.
(modulo the fixup that the linter is asking from you)
Reviewed 1 of 1 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @rauchenstein)
pkg/util/log/file_log_gc_test.go, line 162 at r2 (raw file):
const newLogFiles = 20 for i := 1; i < newLogFiles; i++ { logFn(context.Background(), fmt.Sprint(i))
why did this need to change?
|
after this is green, please issue the backport PR even if we're not going to merge it straight away. I'm somewhat expecting that the backport via the blathers bot won't be clean so it will be an opportunity for you to learn how to use the |
|
install backport via |
This fixes a regression. The call the routine that deletes old log files when max-group-size is configured was incorrectly removed in a refactor, so that functionality was disabled. This change reinstates it. Release note (bug fix): File logs now respect the max-group-size configuration parameter again. This parameter had incorrectly become ignored in v21.1, resulting in arbitrarily large log directories.
|
bors r=knz |
|
Build succeeded: |
|
@rauchenstein henceforce, please be careful to add |
|
Also don't forget the backport |
Fixes #68259
This fixes a regression. The call the routine that deletes old log
files when max-group-size is configured was incorrectly removed in a
refactor, so that functionality was disabled. This change reinstates
it.
Release note (bug fix): File logs respect max-group-size configuration.