Skip to content

Print warning message when page cache thrashing is detected#1605

Open
JaewonHur wants to merge 2 commits into
apple:mainfrom
JaewonHur:warning-on-thrashing
Open

Print warning message when page cache thrashing is detected#1605
JaewonHur wants to merge 2 commits into
apple:mainfrom
JaewonHur:warning-on-thrashing

Conversation

@JaewonHur
Copy link
Copy Markdown
Contributor

Container build process can (seemingly) hang when a lots of threads contend for the disk IO, thrashing each other's page cache. User had no means to tell the hang is due to this issue or the issue in Dockerfile itself. This PR makes build procedure print warning message on thrashing is detected.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Help users root causing the hang while container build

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

Container build process can (seemingly) hang when a lots of threads
contend for the disk IO, thrashing each other's page cache. User had no
means to tell the hang is due to this issue or the issue in Dockerfile
itself. This PR makes build procedure print warning message on thrashing
is detected.
@github-actions github-actions Bot added the cli label May 27, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

Code Coverage

Tier Line Coverage
Unit 33.72%
Integration 19.44%
Combined 52.61%

Copy link
Copy Markdown
Contributor

@jglogan jglogan left a comment

Choose a reason for hiding this comment

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

Approved but see comment.

while true {
let stdout = Pipe()

let process = try await client.createProcess(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Any throwing non-cancel error in here will kill the build, I think. Is this ok?

@dcantah
Copy link
Copy Markdown
Member

dcantah commented May 29, 2026

We have container.stats() today which already tallies up this info, but just doesn't surface these two specific stats. Can we just make a change over in CZ to actually expose them?

@dcantah
Copy link
Copy Markdown
Member

dcantah commented May 31, 2026

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.

3 participants