ci: collect telemetry on every arch, and retry teardown on any FileSystemException - #627
Draft
oyvindberg wants to merge 4 commits into
Draft
ci: collect telemetry on every arch, and retry teardown on any FileSystemException#627oyvindberg wants to merge 4 commits into
oyvindberg wants to merge 4 commits into
Conversation
added 2 commits
July 30, 2026 01:15
The telemetry added in #623 was only ever wired into the `build` job. The native-image matrix -- the only place Windows and macOS run -- had neither step, so those arches produced no metrics and no server log. Windows is where the platform-specific failures actually live, and it was the one arch we could not see into: when its job hung for 36 minutes and was killed by the 40-minute ceiling there was nothing to read afterwards. The collector's socket-dir glob was Linux-only too. `UserPaths.fromAppDirs` takes the cache dir from `ProjectDirectories.from("build", null, "bleep")`, which resolves differently per OS: linux ~/.cache/bleep/socket macos ~/Library/Caches/build.bleep/socket windows %LOCALAPPDATA%\bleep\cache\socket So even had the steps been present, they would have collected nothing off ubuntu. Verified by running the collector on macOS: it now finds three socket dirs the old glob missed. Moved the inline block into .github/scripts/collect-bsp-diagnostics.sh so both jobs share one implementation, globbing all three locations. Windows paths go via $HOME rather than $LOCALAPPDATA because under Git bash the latter is a backslash path and backslashes are escapes to the globber. python3 is now optional -- the raw files are copied before the summary runs, so a missing interpreter costs the convenience summary rather than the diagnostics. Artifacts are named per job (`bsp-diagnostics-build`, `bsp-diagnostics-<matrix.os>`) since six jobs now upload them.
…tEmpty #626 retried the delete only on DirectoryNotEmptyException, which is how the race presents on Linux. Windows presents the same race as a blocked unlink instead, so #625 went from failing PlatformCancellationTest to failing TimeoutAndResourceTest: java.nio.file.FileSystemException: C:\...\Temp\scalajs-mid-cancel...: The process cannot access the file at Files.deleteIfExists at TestFiles$package$$anon$1.postVisitDirectory(TestFiles.scala:55) One cause, two symptoms: a cancelled-but-still-running toolchain either puts a file back after the walk emptied the directory (ubuntu) or holds a handle that blocks the unlink (windows). Both are FileSystemException, so retry on that rather than on either leaf type. Budget raised to 20 × 100ms, enough for a killed node or kotlinc to exit and drop its handles. TimeoutAndResourceTest and PlatformCancellationTest both green locally.
oyvindberg
marked this pull request as draft
July 29, 2026 23:23
added 2 commits
July 30, 2026 08:41
…m to be inferred
Every question asked of this data so far has needed the machine's RAM and the
server's heap cap, and neither was recorded. The temptation is to recover them
from total_memory_mb, and that is wrong twice over:
- total_memory_mb is the *fork* budget -- forkMemoryBudgetMb subtracts the
server's own footprint and an OS reserve from physical RAM. Reading it as
machine RAM makes the heap cap look like it exceeds the machine.
- retuneMemoryBudget then moves it during the run, so the sampled value is
rarely the initial one and inverting the formula gives a number that is
simply wrong. Doing exactly that produced '24GB' for a 16GB runner.
used_memory_mb exceeding total_memory_mb is likewise expected, not an accounting
bug: retuning down while reservations are held does that by construction.
So record physical_memory_mb and server_heap_mb outright. Constant per process
and mildly redundant on a 15s sample, which is the price of a sample that
describes itself. The summariser prints both, and skips the line for older
metrics.jsonl that lack them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two CI-observability/robustness fixes, both surfaced while chasing why Windows kept failing.
1. Telemetry was only ever collected on ubuntu
The compile-server telemetry added in #623 was wired into the
buildjob alone.build-native-image— the only place Windows and macOS run — had neither step, so those arches produced no metrics and no server log. Windows is where the platform-specific failures actually live, and it was the one arch we could not see into: when its job hung for 36 minutes and got killed by the 40-minute ceiling, there was nothing to read afterwards.The collector's glob was Linux-only as well.
UserPaths.fromAppDirstakes the cache dir fromProjectDirectories.from("build", null, "bleep"), which resolves differently per OS:~/.cache/bleep/socket~/Library/Caches/build.bleep/socket%LOCALAPPDATA%\bleep\cache\socketSo even with the steps present it would have collected nothing off ubuntu. Verified by running the collector on macOS: it finds three socket dirs the old glob missed.
Moved the inline block into
.github/scripts/collect-bsp-diagnostics.shso both jobs share one implementation. Windows paths go via$HOMErather than$LOCALAPPDATAbecause under Git bash the latter is a backslash path and backslashes are escapes to the globber.python3is now optional — raw files are copied before the summary runs, so a missing interpreter costs the convenience summary, not the diagnostics. Artifacts are named per job (bsp-diagnostics-build,bsp-diagnostics-<matrix.os>) since six jobs now upload them.2. Teardown retry was too narrow
#626 retried the recursive delete on
DirectoryNotEmptyException— how the race presents on Linux. Windows presents the same race as a blocked unlink, so #625 simply moved from failingPlatformCancellationTestto failingTimeoutAndResourceTest:One cause, two symptoms.
Outcome.runInFreshThreadreturnsThreadOutcome.Cancelledwithout waiting for the worker — it parks it inrunawayThreadsbecause "most native compilers ignore interrupt and keep running". The still-live toolchain then either puts a file back after the walk emptied the directory (ubuntu) or holds a handle that blocks the unlink (windows).Both are
FileSystemException, so retry on that rather than either leaf type. Budget raised to 20 × 100ms — enough for a killed node/kotlinc to exit and drop handles, short enough that a genuinely stuck tree still fails the test instead of hanging the suite.Verification
TimeoutAndResourceTest(8) andPlatformCancellationTest(7) green locally;bleep-bsp-testscompiles; collector exercised on macOS against real socket dirs. The Windows half is what CI has to confirm — that's rather the point of change 1.🤖 Generated with Claude Code