journald logger: fix race condition #10431
Conversation
openshift-ci
bot
added
do-not-merge/work-in-progress
|
WIP for now as I still need to add some errors checks to enforce the journald log/event requirement. I also want to go through the tests and run a bunch of tests with the journald logger. |
|
@mheon @edsantiago PTAL if you find time. Early feedback is good feedback. The reproducer from #10323 is running well on my machine. |
|
I can't look closely right now, sorry. Can you try clearing this and re-pushing? podman/test/system/130-kill.bats Lines 11 to 12 in e48aa8c |
|
The origami continues :^) --events-backend is ignored (after validation) and always set to the defaults from containers.conf. Will spin up another PR to address that first. EDIT: My bad; the events-backend is not stored in the container. |
vrothberg
force-pushed
the
journald-logs
branch
2 times, most recently
from
31576ae to
5a38974
Compare
|
@edsantiago, can you take a look at the system-test changes? |
vrothberg
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@containers/podman-maintainers PTAL |
Fix a race in journald driver. Following the logs implies streaming until the container is dead. Streaming happened in one goroutine, waiting for the container to exit/die and signaling that event happened in another goroutine. The nature of having two goroutines running simultaneously is pretty much the core of the race condition. When the streaming goroutines received the signal that the container has exitted, the routine may not have read and written all of the container's logs. Fix this race by reading both, the logs and the events, of the container and stop streaming when the died/exited event has been read. The died event is guaranteed to be after all logs in the journal which guarantees not only consistencty but also a deterministic behavior. Note that the journald log driver now requires the journald event backend to be set. Fixes: containers#10323 Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
The flake is stubborn |
|
Six flake restarts later, tests are green. Tests LGTM. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Fix a race in journald driver. Following the logs implies streaming
until the container is dead. Streaming happened in one goroutine,
waiting for the container to exit/die and signaling that event happened
in another goroutine.
The nature of having two goroutines running simultaneously is pretty
much the core of the race condition. When the streaming goroutines
received the signal that the container has exitted, the routine may not
have read and written all of the container's logs.
Fix this race by reading both, the logs and the events, of the container
and stop streaming when the died/exited event has been read. The died
event is guaranteed to be after all logs in the journal which guarantees
not only consistencty but also a deterministic behavior.
Note that the journald log driver now requires the journald event
backend to be set.
Fixes: #10323
Signed-off-by: Valentin Rothberg [email protected]