[#1036] Assert that a parked change is handed out, not which thread hands it out

[vharseko]

Fixes #1036.

Two tests of #958 - aChangeHandedOutAsADependencyIsGivenBackWhenItsReplayIsUnwound and
theChangesParkedBehindAChangeWhoseAckFailedAreReplayed - assert isSameAs(replayingParent):
the child parked behind the parent must be replayed by the thread which committed the parent.
getNextUpdate() never guaranteed that. It hands a parked change to whichever replay thread calls
it first once the changes before it have left pendingChanges, and the thread which parked the
child calls it on its own way out, right after addDependency(). The parent commits in the
post-operation plugin, inside op.run(), so a parker delayed past the parent's write finds the
parent gone and takes the child back itself. That is the 0.26 s failure on #981 and #964 - and,
because the assertion sits before the by-hand redelivery of a child no replication server owns,
the child then stays uncommitted at the head of pendingChanges and the remaining 17 tests of the
class time out behind it.

The change

Test tree, plus one javadoc:

• Both tests wait for the child to be seen parked (dependent-changes-size) before the parent
  is released. That is the property they are about - a parked change leaves by getNextUpdate()
  and by no other road - and it closes a second hole the thread check hid: released on the spot,
  the parent could commit before the child was taken off the queue at all, and the child would be
  replayed from the queue with nothing to wait for, a pass which proves nothing.
• The assertion on which thread hands the child out is dropped, and the javadoc of both tests says
  why. The Replication: an Error in a replay kills a replay thread the pool never replaces #923 liveness assertion is made on the thread which met the Error, whichever one it was.
• getNextUpdate()'s javadoc says what it guarantees: first caller once the changes before it are
  gone - as a rule the thread which cleared the dependency, but the parker itself when the clearing
  landed before it got there.

Testing

run	result
mutant: the parker reaches getNextUpdate() 300 ms late, every other caller 500 ms late - original tests	🔴 2/2 - Replica replay thread 1 vs thread 0 "to refer to the same object", the CI failure verbatim, second test cascading
same mutant - tests of this PR	🟢 2/2 - the parker takes the child back, which is the road CI failed on
no mutant, whole UpdateOperationTest	🟢 31/31, 129 s

The mutant is not part of the PR. The branch sits directly on master at 776339a, one commit.

[vharseko]

@maximthomas could you take this one ahead of the queue? It is test-only - two assertions dropped, two waits added, one javadoc - one commit on master, MERGEABLE, 24/24 checks green - and the failure it fixes has become the single largest cost of CI on the replication PRs.

What it is costing

The issue was filed on two hits (#981, #964, 2026-09-12). Since then the same assertion has taken down a leg on every day the replication PRs ran, on branches which touch nothing near it:

day	PR under test	leg	first failure, then the cascade
09-15	#982	ubuntu-latest, 26	thread 20 vs 19 at 0.38 s, 14 of 35
09-15	#1019	ubuntu-latest, 11	thread 20 vs 19 at 0.27 s, 18 of 31
09-15	#1049	ubuntu-latest, 26	thread 21 vs 19 at 0.24 s, 18 of 32
09-16/17	#982	ubuntu-latest, 26	thread 20 vs 19 at 0.26 s, 15 of 39
09-16/17	#988	ubuntu-latest, 21 and 26	thread 20 vs 19 at 0.24 s, 20 of 35 on both
09-16/17	#1019	ubuntu-latest, 25	thread 20 vs 19 at 0.26 s, 17 of 33
09-16/17	#1057	ubuntu-latest, 26	thread 20 vs 19 at 0.25 s, 18 of 33

Ten legs in five days. Every one of them is the same shape - aChangeHandedOutAsADependencyIsGivenBackWhenItsReplayIsUnwound red in a quarter of a second on isSameAs(replayingParent), and because that assertion sits ahead of the by-hand redelivery, the child stays uncommitted at the head of pendingChanges and the rest of UpdateOperationTest times out behind it: 15 to 20 cases at 60 to 250 s each, 19 to 31 minutes of a leg spent on nothing, then the whole 2-hour leg run again. #1019 and #1057 do not touch LDAPReplicationDomain or the replay at all; they were hit because UpdateOperationTest runs in every leg.

Two things make it worse than a flake to re-run. The reruns are what a reviewer sees first: a red UpdateOperationTest on a replication PR is where a real regression would show, and a class which is red once a day for a known reason is one nobody reads closely anymore. And it is not independent of the queue: #985 and #988 add cases to the same class, and every rebase of the stack re-runs the roulette on every leg.

What the PR does

Nothing in main moves but a javadoc. getNextUpdate() hands a parked change to whichever thread calls it first once the changes ahead of it are gone; the two #958 cases asserted that this is the thread which committed the parent, which it usually is and never was guaranteed to be - the parker itself calls getNextUpdate() on its way out, and on a loaded runner it gets there after the parent's write. The cases now wait for the child to be seen parked before the parent is released, which is the property they are about and closes a second hole the thread check hid, and assert that the child is handed out rather than by whom. The mutant which reproduces the CI failure verbatim is in the description: red 2/2 on the original cases, green 2/2 on these.

Every day this waits is another row in that table.

[maximthomas]

praise: The diagnosis is right and the new wait pins what it claims.

• getNextUpdate() never guaranteed the thread: the parker reaches LDAPReplicationDomain.replay()'s tail with dependency=true (the while at :2777 skipped, the finally at :3147 gating only processUpdateDone) and takes the child back itself when the commit landed first — the new javadoc at RemotePendingChanges.java:575-580 matches the code clause by clause.
• "Seen parked ⇒ off the queue" holds by construction: dependentChanges is written only by addDependency() from checkDependencies() on a dequeued message and drained only by getNextUpdate() (and clear() on disable), and initialDependent + 1 is stable while the parent is held (the parker's own getNextUpdate() returns null at RemotePendingChanges.java:644-648).
• The Testing table shows the original assertions red 2/2 under a mutant the PR does not carry — the CI failure verbatim.

---

issue (blocking): The #923 liveness assertion is made on the thread which ran the by-hand redelivery, not on the thread which met the Error.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3136, :3062-3068, :3120-3124

replayingChild is written by the ShortCircuitPlugin predicate on every replayed ADD carrying childCsn: errorFor() (ShortCircuitPlugin.java:851-863) evaluates matches.test(op) before the maxTimes budget check, so the side effect fires on the budget-spent replay too. The redelivery at :3120-3124 runs while thrown is still registered (deregister() is in the finally, after the assertion), so at :3136 the reference names the thread which just applied the child — alive by construction. Measured with the #923 mechanism removed (catch (Throwable t) → catch (Exception t) in ReplayThread.java:133): BASE red 1/1 on isAlive (:3118, the isSameAs at :3085 passed on that run; server log msgID=140 ... Replica replay thread 0 ... terminate abnormally ... LinkageError: the replay of this change is unwound once its ack is out), HEAD green 1/1 with every assertion held. The pin the description's third bullet and the javadoc at :3012-3014 describe is gone.

// UpdateOperationTest.java:3067 — the first replay of the child is the one which meets the Error;
// the redelivery must not overwrite it
replayingChild.compareAndSet(null, Thread.currentThread());

Pin: the same mutant at HEAD, expected red 1/1 on :3136.

---

thought (non-blocking): A red exit of aChangeHandedOutAsADependencyIsGivenBackWhenItsReplayIsUnwound before the by-hand redelivery still wedges the class — the PR removes the #1036 trigger, not the cascade.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3085, :3096-3102, :3139-3143, :3617-3632

On any red before the loop at :3115-3126 the finally runs thrown.deregister(); parked.deregister(): the parent is let through and commits, the child is handed out and applied, processUpdateDone() reads isAssured() (ackPublished = true), getCSN() throws regardless of the ShortCircuit registry, the child is given back uncommitted at the head of pendingChanges and nothing redelivers it — the 14-20 later cases time out as in #1036. The new wait is near-deterministic, so the likelier red exit stays the pre-existing thrownCount wait. Your call on scope; if wanted, a cover(childCsn)-guarded redelivery in the finally leaves the class clean whichever assertion went red.

---

suggestion (non-blocking): The queue arm is exercised by no measured run.

PR description, Testing table

The mutant delays getNextUpdate() callers only (parker 300 ms, others 500 ms) — the parker arm (child ADD REQ before parent ADD RES). The queue arm (parent RES before child REQ, the 2026-09-14 #981 job), which the dependent-changes-size wait is written for, was never seen converted into a parked child. One run would close it:

// LDAPReplicationDomain.replay(), before checkDependencies() — mutant, not for the PR:
// the child is dequeued, then sleeps past the parent's commit
Thread.sleep(500);

Expected: original tests red (child replayed from the queue, isSameAs fails unless the parent's thread dequeues it), PR tests green, access log showing parent RES before child REQ only under the originals.

---

suggestion (non-blocking): The rule the PR refutes survives verbatim at three sites.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java:93-94, :673-674; opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3000-3002

"the parked ones are handed to whichever thread clears what they wait for" (class javadoc), "a parked one is handed to the thread which clears what it waits for" (addDependency()), and the first paragraph of the second test's javadoc, ten lines above the paragraph the PR rewrote: "handed out by getNextUpdate() to the thread which cleared what it was waiting for, and that thread owns it from then on". The give-back conclusions stay true; the premise is the one the new getNextUpdate() javadoc drops.

// e.g. RemotePendingChanges.java:673-674
 * parked one is handed to whichever thread calls getNextUpdate() first once the changes
 * before it are gone - the clearing thread as a rule, the parker itself when the clearing
 * landed first - and one which is not listed here anymore is gone with the pending changes