sd-journal: allow to specify compression algorithm through env #27126
Conversation
github-actions
bot
added
tests
util-lib
please-review
This comment was marked as outdated.
This comment was marked as outdated.
| if (!e) | ||
| return DEFAULT_COMPRESSION; | ||
|
|
||
| r = parse_boolean(e); |
There was a problem hiding this comment.
This makes certain sense, but is it really needed? We already have a config. file option (Compress=) for this, so it doesn't bring anything new. IMHO the new env. var. should be used just for selection of the compression alg. (maybe rename it to SYSTEMD_JOURNAL_COMPRESSION_METHOD or so?)
There was a problem hiding this comment.
This is mostly for COMPRESSION_NONE. We can set it with "NONE", but maybe arbitrary negative boolean value is helpful.
Also, we sometimes have redundant options: e.g. bootctl --esp-path and $SYSTEMD_ESP_PATH.
There was a problem hiding this comment.
This is mostly for
COMPRESSION_NONE. We can set it with "NONE", but maybe arbitrary negative boolean value is helpful.
But it also accidentally accepts yes/no. And how useful is allowing none anyway? It requires editing of a conf. file just like the existing way, and it's longer: Environment=SYSTEMD_JOURNAL_COMPRESSION=none vs. Compress=no.
Also, we sometimes have redundant options: e.g. bootctl --esp-path and $SYSTEMD_ESP_PATH.
Yes, but these have a different scope. The command line option is for one-off use, while the env. var. can be set in environment.
There was a problem hiding this comment.
I mean, I don't really care that much. It just seems to me you're overthinking this a bit.
There was a problem hiding this comment.
Note, the scope is different. Compress=yes|no works only for journald. But SYSTEMD_JOURNAL_COMPRESS=yes|no|XZ|LZ4|ZSTD also works journal-remote.
(Maybe, we should also introduce Compress=yes|no for journal-remote.conf later.)
|
lgtm, just nitpicks, ultimately |
Previously, data object may be compressed with an algorithm that is not mentioned in the header.
Fixes RHBZ#2183546 (https://bugzilla.redhat.com/show_bug.cgi?id=2183546). Previously, journal file is always compressed with the default algorithm set at compile time. So, if a newer algorithm is used, journal files cannot be read by older version of journalctl that does not support the algorithm. Co-authored-by: Colin Walters <walters@verbum.org>
YHNdnzj
added
good-to-merge/waiting-for-ci 👍
test/units/testsuite-04.sh
Outdated
| journalctl --rotate | ||
|
|
||
| ID=$(systemd-id128 new) | ||
| systemd-cat -t "$ID" /bin/sh -c "for ((i=0;i<100;i++)); do echo -n hoge with ${c}; done; echo" |
There was a problem hiding this comment.
for i in {1..100}; do …
The logs in ubuntuautopkgtests fail with:
Apr 04 20:37:26 H 6c9593a7ec374bb095fba079bf38e79f[844]: /bin/sh: 1: Syntax error: Bad for loop variable
I don't know if this this line, but it seems the most likely. But we'd want the normal syntax anyway.
There was a problem hiding this comment.
Changed to /bin/sh -> /bin/bash. Hopefully now it works.
keszybz
added
ci-fails/needs-rework 🔥
…ompression algorithm
github-actions
bot
added
please-review
yuwata
added
the
good-to-merge/waiting-for-ci 👍
|
jammy-ppc64el: TEST-66-DEVICE-ISOLATION: FAIL (1201 s), unrelated. |
keszybz
added
ci-failure-appears-unrelated
and removed
good-to-merge/waiting-for-ci 👍
| static const unsigned supported = | ||
| (1U << COMPRESSION_NONE) | | ||
| (1U << COMPRESSION_XZ) * HAVE_XZ | | ||
| (1U << COMPRESSION_LZ4) * HAVE_LZ4 | | ||
| (1U << COMPRESSION_ZSTD) * HAVE_ZSTD; |
There was a problem hiding this comment.
At -O0 this generates quite a long function, because supported is not inlined. But at -O2 it works nicely:
000000000010daa0 <compression_supported>:
(1U << COMPRESSION_NONE) |
(1U << COMPRESSION_XZ) * HAVE_XZ |
(1U << COMPRESSION_LZ4) * HAVE_LZ4 |
(1U << COMPRESSION_ZSTD) * HAVE_ZSTD;
return c >= 0 && c < _COMPRESSION_MAX && FLAGS_SET(supported, 1U << c);
10daa0: 83 ff 03 cmp $0x3,%edi
10daa3: 0f 96 c0 setbe %al
}
10daa6: c3 ret
10daa7: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
10daae: 00 00
👍
With the latest fix in systemd [1], we can use an environment variable to tell journald to use a specific compression algorithm for the journal and thus don't need a custom build anymore. See: systemd/systemd#27126 [1] See: openshift#1230 See: https://issues.redhat.com/browse/LOG-3832 See: https://bugzilla.redhat.com/show_bug.cgi?id=2183546
With the latest fix in systemd [1], we can use an environment variable to tell journald to use a specific compression algorithm for the journal and thus don't need a custom build anymore. See: systemd/systemd#27126 [1] See: openshift#1230 See: https://issues.redhat.com/browse/LOG-3832 See: https://bugzilla.redhat.com/show_bug.cgi?id=2183546 (cherry picked from commit dd2059d)
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2183546