Open gibson042 opened 4 days ago
I am very confused by what https://github.com/nodejs/node/pull/50009 attempts to fix. A fsync
should only be necessary if you attempt to read from another OS, e.g. network mounted system, or if your computer crashes. I believe the linux kernel guarantees that the same file being read by another process will use the most recently written data, even if it wasn't committed to disk.
From a conversation on Slack, it's a lot more likely that ava concurrency results in the same config file being written by concurrent tests, potentially causing tears since the file being written may take multiple write
syscalls.
The solution is likely to add a random component to testConfigPath
I am very confused by what nodejs/node#50009 attempts to fix. A
fsync
should only be necessary if you attempt to read from another OS, e.g. network mounted system, or if your computer crashes. I believe the linux kernel guarantees that the same file being read by another process will use the most recently written data, even if it wasn't committed to disk.
There are several things to consider even when a single file is used by a single process on a single host:
With that in mind, only option 1 could be applicable to a simple config file write/read by a single process, and only in case of application crash/abort.
Describe the bug
As seen at https://github.com/Agoric/agoric-sdk/actions/runs/10873209314/job/30169121954?pr=10091#step:5:1
I suspect this can be fixed by adding aflush: true
option to thewriteFile
call in packages/boot/tools/supports.ts, but only after raising the Node.js version bar to at least v20.10.0 (cf. https://github.com/nodejs/node/pull/50009 for background). And if so, then we may want to consider using it in even morewriteFile
s. A possible alternative would be reading back the expected size, but that seems too heavyweight.Subsequent conversation on Slack identified a more likely culprit as concurrently running tests writing and reading against the same file name, a hypothesis which was locally reproducible:
We should instead introduce a random component in these Swingset config file names.