Skip to content

fix(runner): accept earlyoom's SIGKILL in the memory-hog subtest - #7005

Merged
kixelated merged 3 commits into
mainfrom
quest/m0/test-flakes-2/runner-vm
Oct 28, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m0/test-flakes-2/runner-vm

Conversation

@kixelated

@kixelated kixelated commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

checks.x86_64-linux.github-runner-vm failed in two merge groups for moq-dev/moq#6934 (40609795466, 40619259411). The full log from nix log on the runner host names the failing subtest:

subtest: a job that fills memory is killed before it thrashes the disk
earlyoom[501]: mem avail:    30 of   745 MiB ( 4.14%), swap free:    0 of    0 MiB ( 0.00%)
earlyoom[501]: low memory! at or below SIGKILL limits: mem  5.00%, swap  5.00%
earlyoom[501]: sending SIGKILL to process 903 uid 0 "python3": ... cmdline ".../hog.py /var/tmp/hog-inputs"
memory-hog failed after 4s: read 0 MiB, memory pressure avg10=0.00
!!! RequestedAssertionFailed: command `journalctl -u earlyoom | grep -F 'sending SIGTERM to process'` failed (exit code 1)

Root cause. The guest did what the subtest is for: earlyoom killed the hog after 4s, with 0 MiB re-read from disk. The test then required the word SIGTERM. earlyoom sends SIGTERM only when a poll lands between 10% and 5% available, and SIGKILL when it first lands below 5%. On the runner host the hog filled 700 MiB in 4s and crossed that 37 MiB band between two polls; locally it took about 8s and earlyoom saw 9.7%, so it sent SIGTERM. Which signal is sent depends on the hog's fill rate against earlyoom's poll interval. It is not a property of the runner, so the runner host's state (store GC, KVM, memory) does not matter beyond letting the check rebuild. The result rebuilt because the host's weekly nix-gc on 2026-10-26 collected it, and a failed build is not cached, so the second run rebuilt and failed the same way.

Fix.

  • The subtest now asserts that earlyoom sent SIGTERM or SIGKILL to the hog itself, matched by hog.py's store path. That is stricter about the target than before (any SIGTERM matched) and accepts either of earlyoom's two kill paths. The 60s no-thrash deadline and the sshd check are unchanged.
  • just ci/just check run nix flake check --print-build-logs, so the next failing VM subtest prints in full instead of the last 25 lines of QEMU cleanup. Logs grow only when a check actually rebuilds, which is when they are needed.

Checks. The VM test passes locally (all 10 subtests, 183s; earlyoom sent SIGTERM there). The new pattern matches the runner's SIGKILL line from the failing log. just check passes except check flake, which skips locally because this Nix builder cannot create user namespaces. That is why I built the VM test directly. sh/dispatch-query.test.py is updated for the new flag.

Public API: none. Wire: none.

Finishes and removes quest/m0/test-flakes-2/runner-vm.md (added by moq-dev/moq#6998).

Choices made without asking: accepting either signal rather than forcing SIGTERM (which signal earlyoom sends is not a guarantee the runner needs), and --print-build-logs over parsing nix log from the error output (simpler; the extra output appears only when a check rebuilds).

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

earlyoom sends SIGKILL instead of SIGTERM when its poll first lands below
the 5% kill threshold, so the subtest's SIGTERM grep failed whenever the
hog filled memory faster than earlyoom polled. Assert that earlyoom killed
the hog with either signal, and print full build logs from the flake check
so a failing VM subtest names itself.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T05:22:48.136322Z af18985 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 19 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3271de4d-433c-49b6-98f9-741b75dde8e8
Commits

Reviewing files that changed from the base of the PR and between bea4214 and af18985.

Files selected for processing: 5
  • nix/runner/test.nix
  • quest/m0/test-flakes-2/README.md
  • quest/m0/test-flakes-2/runner-vm.md
  • sh/dispatch-query.test.py
  • sh/dispatch.sh
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: 8a7245a

No actionable bugs found in the changed areas. Accepting either earlyoom signal while matching the hog's store path is a focused fix: it removes the timing-dependent signal assumption without relaxing the victim check, deadline, or SSH recovery check. The build-log flag and corresponding dispatcher expectations stay aligned; no extra abstraction is needed.

Verification: static review of the diff and surrounding VM test, hog workload, launcher, and dispatcher tests only. I did not run the Python suite or Nix VM test, or independently reproduce the SIGKILL case.

kixelated and others added 2 commits October 27, 2026 22:21
@kixelated
kixelated added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 0c2bd2c Oct 28, 2026
12 checks passed
@kixelated
kixelated deleted the quest/m0/test-flakes-2/runner-vm branch October 28, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant