Concurrent password hashing adds tail latency — measured, and not worth acting on at this traffic #163
No Assignees
bermudalamb
bermudalamb
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bermudalamb/redefined-designs#163
Reference in New Issue
Block a user
This body has been rewritten. The original argued that
bcryptjsblocks the event loop and that every login and registration stalls every other request in flight. That was asserted without measurement and it is false — see the first comment for the retraction. The issue is kept, rather than closed outright, because the smaller effect underneath it was worth a number.What is actually true
bcryptjshas two APIs.hashSyncandcompareSyncblock. The asynchronous API chunks its work and yields between rounds, and the asynchronous API is what all six call sites inroutes/customers.tsuse. There is nohashSyncorcompareSyncanywhere insrc.So the server keeps serving during a hash. What it does not do is hash in parallel: the work is still CPU on the one thread, so N simultaneous hashes serialize, and the chunks are coarse enough that a request arriving at the wrong moment waits.
The measurement
backend/scripts/bench-hash-latency.ts, run asnpm run bench:hashing. The probe isGET /api/customers/mewith no session cookie, chosen because it rejects before touching the database — nearly all of its latency is waiting for the event loop rather than work of its own. The load is real registrations against the real route.Dev machine, Node 24, cost 12:
Both effects are linear in the number of queued hashes:
The conclusion
Not worth acting on. This is a homelab storefront; eight simultaneous registrations is not a load it will ever see, and at one or two the cost is invisible. Every option in the original issue — native
bcrypt,argon2, a worker pool — buys a real improvement against a problem that is not currently costing anything, and the two native options add anode-gyptoolchain to a Dockerfile that currently needs none, on a NAS, built by CI. That is a live risk traded for a theoretical gain.The number that would change this answer is the registration one. If real users ever register concurrently enough for 1.6 s to matter, or if the end-to-end suite is shown to be timing out on registrations specifically, re-run the benchmark and revisit. That is why it is committed rather than pasted here.
What this no longer supports
This issue was cited as the second, independent cause of the end-to-end suite's instability, and as a reason #116 would not fix it alone. That reasoning rested on the refuted premise. It is also worth noting that the flakiness observed since — assertions against the unfiltered storefront grid — was traced to something else entirely: a development database that nothing truncates, now holding ~1600 items rendered unpaginated, so a 5 s timeout expires before the grid finishes. Registrations at 1.6 s are well inside Playwright's 5 s default and do not by themselves explain a timeout.
Recommendation
Close. The question is answered, the answer is "no", and the benchmark is in the repository so the answer can be checked rather than remembered.
This issue's premise is wrong. Measured, not argued.
The body claims
bcryptjsblocks the event loop and that "every login and registration stalls every other in-flight request". That is false, and it was asserted without measurement.bcryptjshas two APIs.hashSync/compareSyncblock. The asynchronous API chunks its work and yields between rounds, and the asynchronous API is what all six call sites inroutes/customers.tsuse.Measured on the dev machine, Node 24, cost 12:
bcrypt.hashSyncawait bcrypt.hashA timer scheduled for 0 ms cannot fire while the loop is blocked, so how late it fires is the block duration — no polling and no interval granularity involved, which matters because Windows timer resolution made a first attempt at this unreadable.
The server keeps serving during a hash. It is not a throughput ceiling.
What is actually true
A 102 ms gap on a 187 ms hash means the chunks are coarse. Under concurrency that is real added latency, and eight simultaneous registrations still queue roughly 1.5 s of CPU that has to come from somewhere. So there is a smaller, real effect — a latency characteristic worth knowing about, not a stall.
Whether that is worth acting on is now an open question rather than a settled one. It probably is not, at this traffic, without evidence of it hurting something.
The claim this invalidates elsewhere
This issue has been cited repeatedly as the second, independent cause of the end-to-end suite's instability, and as a reason #116 would not fix it alone. That reasoning rested on this premise and falls with it. #116 should be approached on its own merits, and if the suite is still unstable afterwards the cause has to be found rather than assumed to be this.
The suite's shifting failures remain unexplained. The shared-database interference in #116 is a demonstrated cause; whether it is the only one is not known.
What this issue should be now
Either closed, or narrowed to the latency question with a measurement bar attached — something like "a request served concurrently with N registrations does not exceed X ms" — so any future change to hashing is justified by a number rather than by the reasoning above, which was wrong.
Recommend narrowing rather than closing: the
--coverage-heavy CI job and an 8-worker Playwright run are the two places where queued hashing plausibly does cost something, and neither has been measured.Password hashing blocks the event loop, stalling every other request in flightto Concurrent password hashing adds tail latency — measured, and not worth acting on at this traffic