Skip to content

Suppress warnings for transient stdin backpressure - #715

Open
TDnorthgarden wants to merge 2 commits into
containers:mainfrom
TDnorthgarden:fix/689-eagain-stdin
Open

TDnorthgarden wants to merge 2 commits into
containers:mainfrom
TDnorthgarden:fix/689-eagain-stdin

Conversation

@TDnorthgarden

Copy link
Copy Markdown
Contributor

Carries #705 — original work by @sairex53, whose Signed-off-by is preserved in both commits. Opened with @kolyshkin's go-ahead after the
original author stayed quiet since the review request of Oct 28.

What this does. Writes to the container stdin pipe and socket are nonblocking; EAGAIN/EWOULDBLOCK mean the bytes stay queued and are
retried when the fd becomes writable. That is expected backpressure, not a failure, so conmon should not warn on it. This stops the
journal flood described in #689 for slow stdin consumers.

Changes on top of #705 (per @kolyshkin's review):

  1. test: move container helpers to test_helper.bash — separate cleanup commit; also removes the create_test_script / update_container_args
    definitions left behind in test/06-priority-parsing.bats
  2. regression test: the container counts its input (wc -c) and the test asserts the exact byte count, so it also catches input loss on
    EAGAIN retry
  3. payload reduced from 8 MiB to 1 MiB
  4. negative check is non-vacuous: asserts the journal output is non-empty first, then uses the assert ... !~ helper

Verification (local, rootless runc + user journal):

  • new test: fails on the unfixed binary (spurious warning in journal), passes with the fix
  • test/07-attach.bats: 4/4 · test/06-priority-parsing.bats: 9/9

Fixes #689.

create_test_script, update_container_args and skip_if_no_journald
were defined in 06-priority-parsing.bats, but the new 07-attach.bats
regression test also needs them. Move them to test_helper.bash and
remove the now-duplicated definitions from 06-priority-parsing.bats.

Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
Signed-off-by: Aucoto Zhang <tdnorthgarden@gmail.com>
Writes to the container stdin pipe and socket are nonblocking. When
they return EAGAIN/EWOULDBLOCK, the pending bytes remain queued and
are retried when the fd becomes writable, so this is expected
backpressure rather than a write failure. Do not log a warning for
those errno values while preserving warnings for other errors.

Previously, a slow stdin consumer made conmon log "Failed to write
to fd container stdin" on every EAGAIN, flooding the journal.

Add an attach regression test that delays the container's stdin
reader, sends a 1 MiB payload, and checks that every byte is
delivered and conmon's journal contains no spurious warning.

Fixes: containers#689
Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
Signed-off-by: Aucoto Zhang <tdnorthgarden@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

conn_sock.c: 'Failed to write container stdin' is logged on every EAGAIN, flooding the journal for slow stdin consumers

1 participant