Page MenuHomeFreeBSD

newsyslog(8): Add option to globally override compression method.
ClosedPublic

Authored by delphij on Dec 22 2023, 8:24 AM.
Tags
None
Referenced Files
F86344358: D43165.diff
Wed, Jun 19, 3:16 AM
Unknown Object (File)
Tue, Jun 4, 4:18 PM
Unknown Object (File)
Tue, Jun 4, 4:18 PM
Unknown Object (File)
Tue, Jun 4, 4:18 PM
Unknown Object (File)
Tue, Jun 4, 4:18 PM
Unknown Object (File)
Tue, Jun 4, 4:18 PM
Unknown Object (File)
Tue, Jun 4, 4:17 PM
Unknown Object (File)
Tue, Jun 4, 4:17 PM

Details

Summary

Historically, newsyslog compressed rotated log files to save disk space. This was
useful in the early days. However, with modern file systems like ZFS offering native
compression, and with the availability of larger hard drives, the benefits of
additional compression have become less significant. This is particularly true
considering the inconvenience of decompressing log files when searching for
specific patterns.

Additionally, the original implementation of compression methods was not future-proof.
As a result, we have redefined the J, X, Y, Z flags to signify "treat the file as
compressible" rather than "compress the file with that specific method."

A new command-line option, -c, has been introduced to allow overriding these settings
in a more future-proof way. The available choices are:

    
none - do not compress, regardless of flag.
legacy - historical behavior: J=bzip2, X=xz, Y=zstd, Z=gzip.
bzip2, xz, zstd, gzip - apply the specified compression method.

Currently, the default is set to 'legacy' to preserve historical behavior. However,
our intention is to change this default to 'none' in FreeBSD 15.0.

Additionally, this update changes the default settings for zstd to use multithreading
and long-range options, better aligning with its intended use.

Inspired by D42961

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 55094
Build 51983: arc lint + arc unit

Event Timeline

delphij created this revision.

Fix incorrect logic in override handling.

usr.sbin/newsyslog/newsyslog.c
98

Will "legacy" type be deprecated and removed in future version ?

mdoc(7) syntax looks fine to me.

This revision is now accepted and ready to land.Dec 22 2023, 2:27 PM

I like this approach. If I read it correctly, changes to newsyslog.conf are not necessary to turn off log file compression. If this is implemented, D42961 is not required.

If that's the case, I like it and prefer it. It is an elegant solution.

zarychtam_plan-b.pwste.edu.pl added inline comments.
usr.sbin/newsyslog/newsyslog.c
142–143

What about adding "-T0", "--adapt", "--long" to this list together with this change ?

delphij marked an inline comment as done.

Add an UPDATING entry, and adjust zstd default settings per reviewer
comment.

This revision now requires review to proceed.Dec 22 2023, 7:13 PM
delphij added inline comments.
usr.sbin/newsyslog/newsyslog.c
98

Will "legacy" type be deprecated and removed in future version ?

I think it's unlikely to happen, but we may undocument the historical behavior and only keep one ("Z" maybe) documented eventually.

142–143

This seems to have improved compression a little bit for my test files (typical mail server log) and I think it's reasonable. I'm not sure if --long --adapt really makes sense for smaller files, though.

The newsyslog tests fail after applying this patch.

The UPDATING entry should appear in RELNOTES instead, since it applies to all users, not just those tracking main.

delphij marked an inline comment as done.

Unbreak legacy tests.

The newsyslog tests fail after applying this patch.

Thanks, fixed.

The UPDATING entry should appear in RELNOTES instead, since it applies to all users, not just those tracking main.

I'll do both, but RELNOTES needs to be added later as it mentioned the SHA1 revision.

otis added inline comments.
usr.sbin/newsyslog/newsyslog.c
681

Is this subject to change in future? If yes, then this string could contain programatically-generate compression names.

delphij marked an inline comment as done.

Dynamically derive usage from compression type table.

usr.sbin/newsyslog/newsyslog.c
681

Good point, thanks. I've moved this to usage() and implemented a dynamic generation from the compression type table.

The newsyslog tests fail after applying this patch.

Thanks, fixed.

The UPDATING entry should appear in RELNOTES instead, since it applies to all users, not just those tracking main.

I'll do both, but RELNOTES needs to be added later as it mentioned the SHA1 revision.

I like both too. UPDATING and RELNOTES target different audiences

Reduce scope of change to only add new functionality in preparation of
separating the default flip in a different commit.

delphij retitled this revision from newsyslog(8): Disable compression by default. to newsyslog(8): Add option to globally override compression method..Dec 23 2023, 3:28 AM
delphij edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.Dec 23 2023, 6:53 AM
This revision was automatically updated to reflect the committed changes.

Some people criticised this change, while others were enthusiastic. What about MFC? The commit message states "MFC after 1 week". It would be nice to MFC this change to stable/14. I have cherry-picked it just after the original commint and thus now get a 25% smaller zstd compressed archives of daily 5 GB log file.