flake: durable_manager.test SemFiresOnlyAfterFlush races its own 300ms flush cadence

[ohohoreilly]

orly/indy/disk/durable_manager.test.cc:183 fails intermittently in CI:

[orly/indy/disk/durable_manager.test.cc, 183](bool)!sem.GetFd().IsReadable(0); !1; fail
end SemFiresOnlyAfterFlush; fail
passed 2, failed 1

Why it is the test, not the product

durable_manager.Save(id, deadline, ttl, blob, &sem);
/* Not yet: the writer flushes on a 300ms cadence, and durability must not be signalled
   before the data is written (#277). */
EXPECT_FALSE(sem.GetFd().IsReadable(0));

The assertion is a race by construction. It requires the test thread to reach IsReadable(0) within the 300ms write-delay window. On a loaded runner, if the thread is descheduled for longer than that between Save() and the check, the flush completes, the semaphore fires legitimately, and the test reports a failure for correct behaviour.

So the property it is guarding (#277: durability must not be signalled before the write lands) is real and worth guarding — the encoding of it is what is unsound. It asserts "has not happened yet" using wall-clock luck.

Evidence that it is transient

Same commit tree, three consecutive runs on PR #550:

run	new commit contents	make debug + make test
34921446403	prefetch + CI job	success
34921609238	-msse2 removal	success
34922824232	README.md + a changelog fragment only	fail

The failing run's only new commit contains no code. The binaries that failed are byte-identical to ones that passed twice. Master's scheduled crons are 18 success / 1 cancelled / 1 null over the last 20.

Why this is worth fixing rather than re-running

It fails in a way that points at the wrong culprit. I hit it on a PR that changes compiler flags and a prefetch intrinsic — the two things most likely to be blamed for a mysterious disk-layer failure — and it cost a full bisect across three runs to establish innocence. The next person gets the same bill, and the standing advice is (rightly) to suspect your own changes before calling anything a flake.

Possible shapes

• Measure, don't assume. Record a timestamp before Save() and only assert !IsReadable if less than the write delay has actually elapsed; otherwise skip the negative assertion (the positive sem.Pop() + TryLoad checks still run).
• Widen the window. The delay is a constructor argument; a much larger write delay for this one case makes descheduling-sized jitter irrelevant. Cheapest change, keeps the assertion meaningful.
• Assert the ordering directly rather than via a timing proxy — e.g. observe that the sem fires only after the write callback, if the manager exposes a seam for that.

The second is probably the right first move: it preserves exactly what #277 was protecting while removing the dependence on scheduler luck.

[ohohoreilly]

Recurred on the aarch64 nightly on 2026-09-19 (run 35435047828), four days after this was closed:

TEST: orly/indy/disk/durable_manager.test
[orly/indy/disk/durable_manager.test.cc, 200](bool)!fired_immediately; !1; fail
end SemFiresOnlyAfterFlush; fail

That's 1 in 14 arm nightlies since then and none on amd64. The arm job is continue-on-error, so the run still reported green. Reopening because the fix doesn't hold on arm.

[ohohoreilly]

Correction to the count above. Going through every failed CI job across the last 300 ci.yml runs, this test failed on the arm job three times: 09-15 (run 34946111193), 09-18 (35299588362) and 09-19 (35435047828). It has not failed on amd64 in that window.

[ohohoreilly]

Root cause found. The flake isn't scheduler jitter: the write delay the test relies on never happens. Util::SleepUntil (base/util/time.h:45) has had its comparison inverted since 2014 and never sleeps. A verbatim copy timed in isolation sleeps 0.000ms for a 2000ms deadline. So TFlush::WaitFor returns at once and the writer flushes the moment a save arrives. The "not fired yet" assertion was a race against that flush, which is why widening the delay to 2s in #553 changed nothing.

• Engine side, filed as Util::SleepUntil never sleeps: replication, merge and durable-write intervals have been no-ops since 2014 #576: every configured cadence (replication_interval, merge_mem_interval, merge_disk_interval, durable_write_interval) has been a no-op. Fixing that is a deliberate, measured change for its own session.
• Test side, PR durable_manager.test: assert sem-implies-on-disk, not a timing race (#551) #577: asserts the indy durability: don't signal completion before data is durably written #277 property in the direction that can't race (sem fired ⇒ the durable file is already registered with the engine). It is being proven against a reintroduced pre-indy durability: don't signal completion before data is durably written #277 synchronous push before merge.