Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(#2029426) Rework PID change handling in journald #273

Merged

Conversation

dtardon
Copy link
Member

@dtardon dtardon commented May 10, 2022

No description provided.

@systemd-rhel-bot systemd-rhel-bot added pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels May 10, 2022
@systemd-rhel-bot systemd-rhel-bot changed the title Rework PID change handling in journald (#2029426) Rework PID change handling in journald May 10, 2022
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels May 10, 2022
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels May 27, 2022
@systemd-rhel-bot systemd-rhel-bot changed the title (#2029426) Rework PID change handling in journald (#2029426) (#2029426) Rework PID change handling in journald Jun 9, 2022
@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-ci Formerly needs-ci label Jun 13, 2022
@systemd-rhel-bot systemd-rhel-bot changed the title (#2029426) (#2029426) Rework PID change handling in journald (#2029426) Jun 13, 2022
@mergify mergify bot added the pr/needs-ci Formerly needs-ci label Jun 13, 2022
@lnykryn lnykryn changed the title (#2029426) (#2029426) Rework PID change handling in journald Jun 15, 2022
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels Jun 20, 2022
@dtardon dtardon force-pushed the bz2029426-journal-newline branch from dfbb965 to 9ba5063 Compare June 23, 2022 12:09
msekletar
msekletar previously approved these changes Jul 12, 2022
Copy link
Member

@msekletar msekletar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@systemd-rhel-bot systemd-rhel-bot removed the pr/needs-review Formerly needs-review label Jul 12, 2022
Copy link
Member

@mrc0mmand mrc0mmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test won't work without systemd/systemd#15648, since currently we ignore --output-fields= option in the cat output mode (-o cat):

Fedora:

$ journalctl -b -o cat --output-fields=_TRANSPORT -n5
stdout
stdout
stdout
stdout
journal

RHEL 8:

# journalctl -b -o cat --output-fields=_TRANSPORT -n5
kdump: Starting kdump: [OK]
Started Crash recovery kernel arming.
Startup finished in 701ms (kernel) + 1.617s (initrd) + 36.679s (userspace) = 38.998s.
systemd-hostnamed.service: Succeeded.
Source 104.131.155.175 replaced with 2601:603:b7f:fec0:0:ba11:ba11:ba11 (2.rhel.pool.ntp.org)

@systemd-rhel-bot systemd-rhel-bot added pr/needs-review Formerly needs-review and removed pr/needs-review Formerly needs-review labels Aug 20, 2022
@jamacku
Copy link
Member

jamacku commented Aug 24, 2022

@dtardon Could you please have a look, thank you.

@systemd-rhel-bot systemd-rhel-bot added tracker/missing Formerly needs-bz tracker/unapproved Formerly needs-acks and removed tracker/missing Formerly needs-bz labels Sep 3, 2022
@systemd-rhel-bot systemd-rhel-bot changed the title (#2029426) Rework PID change handling in journald (#2029426) (#2029426) Rework PID change handling in journald Oct 3, 2022
@systemd-rhel-bot systemd-rhel-bot changed the title (#2029426) (#2029426) Rework PID change handling in journald (#2029426) (#2029426) (#2029426) Rework PID change handling in journald Oct 13, 2022
@jamacku
Copy link
Member

jamacku commented Feb 13, 2023

@dtardon Could you please rebase on the latest master. Thank you

@jamacku jamacku changed the title (#2029426) (#2029426) (#2029426) Rework PID change handling in journald (#2029426) Rework PID change handling in journald Feb 13, 2023
@jamacku jamacku added this to the RHEL-8.8 milestone Feb 13, 2023
benjarobin and others added 5 commits February 13, 2023 19:07
If the previous received buffer length is almost equal to the allocated
buffer size, before this change the next read can only receive a couple
of bytes (in the worst case only 1 byte), which is not efficient.

(cherry picked from commit 034e971)

Related: #2029426
Let's introduce an explicit line ending marker for line endings due to
pid change.

Let's also make sure we don't get confused with buffer management.

Fixes: #15654
(cherry picked from commit 45ba1ea)

Resolves: #2029426
(cherry picked from commit c11d8fd)

Related: #2029426
benjarobin and others added 2 commits February 14, 2023 16:43
Check:
 - There is only 3 messages logged with type stdout
 - Check all messages logged does not have new line: LINE_BREAK=eof
 - Check that the 3 messages are logged from a different PID
 - Check the 3 MESSAGE= content
(cherry picked from commit d38b3b7)

Related: #2029426
(cherry picked from commit a3d9aee)

Related: #2029426
@dtardon dtardon force-pushed the bz2029426-journal-newline branch from 56eccac to c09bb44 Compare February 14, 2023 15:43
@mergify mergify bot removed the pr/needs-ci Formerly needs-ci label Feb 14, 2023
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels Feb 16, 2023
@systemd-rhel-bot systemd-rhel-bot added tracker/unapproved Formerly needs-acks and removed tracker/unapproved Formerly needs-acks labels Feb 25, 2023
@jamacku jamacku modified the milestones: RHEL-8.8, RHEL-8.9 Feb 27, 2023
@jamacku jamacku requested a review from msekletar February 27, 2023 13:00
Copy link
Member

@msekletar msekletar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants