chadtmiller.com/knotless
git that lives in your Atmosphere account
Draft: measure harness PDS latency per method
ClosedThis is a throwaway measurement pull, and its body says it is not for merging. It adds PDSSTAT per-method latency logging to harness/pds.mjs, prints the last 26 PDSSTAT lines of each cov-net-*.log from harness/cov.mjs on exit, and sets COV_ONLY=run,gateway,forge COV_NETS=spaces in .knotless/workflows/test.yml. I read the whole diff and found nothing wrong for its purpose.
I did not check whether readdirSync and readFileSync are imported in harness/cov.mjs, since the diff doesn't show the imports.
If this were ever merged, the COV_ONLY/COV_NETS change would narrow what test runs and weaken the 100% coverage gate. The PDSSTAT hooks would also stay in the harness. Close the pull when you have the numbers.
Since my approval at b42026f, this pull adds an fsync micro-benchmark and a threadpool/CPU line, changes the PDSSTAT printing in cov.mjs to keep the first 3 lines plus the last 26, and wraps the listener target as net.pds.server.server ?? net.pds.server.
The listener change breaks the harness. The statement before it, let inflight = 0, maxInflight = 0, has no semicolon, and the next line now starts with (. JavaScript reads that as 0(...), so the harness throws TypeError: 0 is not a function when pds.mjs starts. Before this change the line began with net, so ASI was not an issue. As it stands the pull produces no numbers. The fix is a leading ; on that line, or a ; after maxInflight = 0.
I did not check that tmpdir and join are imported in pds.mjs. The diff doesn't show the imports.
The rest is fine for a throwaway pull. Close it once you have the numbers.
harness/pds.mjsline 140Outdated
h.enable() | |
const stats = new Map() | |
let inflight = 0, maxInflight = 0 | |
(net.pds.server.server ?? net.pds.server).prependListener('request', (req, res) => { |
The previous line, let inflight = 0, maxInflight = 0, has no semicolon, so this line parses as 0(...) and throws a TypeError. Add a leading ; here.
The leading ; before (net.pds.server.server ?? net.pds.server) fixes the 0(...) TypeError I asked about. The rest of the change is a reformat of the measurement code, plus a new /dev/shm free-space line wrapped in try/catch. I found nothing else wrong for a throwaway pull. Close it once you have the numbers. Merging it would leave the COV_ONLY/COV_NETS narrowing in test.yml and the PDSSTAT hooks in the harness.
Since my approval at e470efb, this pull adds mcp to COV_ONLY in test.yml. It also has the harness keep the PDS's databases in /dev/shm when 2 GiB is free, with KNOTLESS_PDS_DISK=1 as the opt-out. cov.mjs sets that opt-out for the -1 harness, so the two run on different storage and can be compared. It also keeps 5 leading PDSSTAT lines instead of 3, and the PDSSTAT line now reports the disk setting and the temp directory. I found nothing that breaks this. os.tmpdir() reads TMPDIR when it is called, so setting it after the imports works.
One gap, not worth fixing here: the /dev/shm cleanup runs on exit. If the harness is killed by a signal, that handler doesn't run and the directory stays in /dev/shm.
Close it once you have the numbers. Merging it would leave the COV_ONLY/COV_NETS narrowing in test.yml, which weakens the coverage gate. It would also leave the PDSSTAT hooks in the harness.
Since my approval at 45fa925, this pull drops the /dev/shm relocation. In its place, harness/pds.mjs patches the PDS's Database.sqlite to set synchronous: 'OFF', so commits skip the fsync. KNOTLESS_PDS_FSYNC=1 turns the patch off, and cov.mjs sets it for the -1 harness so the two runs can be compared. The PDSSTAT code is trimmed, and docs/development.md is updated to match. The shm cleanup gap I noted earlier no longer applies.
I found nothing that breaks this. I did not check that Database.sqlite is a static method taking (location, opts) with a pragmas option, or that db/db.js exists at that path in the PDS version the harness pins. If either is wrong, the harness throws when it starts, and you'd see that right away. It's the only unverified assumption in the patch.
Close it once you have the numbers. Merging it would leave the COV_ONLY/COV_NETS narrowing in test.yml, which weakens the coverage gate. It would also leave the PDSSTAT hooks in the harness.
What changes
Throwaway timing of the harness PDS per XRPC method. Not for merging.
Changeset: none