[#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 javadoc and comments in main:

• 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
  • and only on that one: the ShortCircuitPlugin predicate which records it runs on the by-hand
    redelivery of the child as well, so the record is a compareAndSet(null, …) rather than a set
    (review round 2).
• 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. The rule this replaces - "a parked change is handed to the thread
  which clears what it waits for" - is rewritten wherever it was stated: the class javadoc,
  addDependency() and getChangeOwnedByCurrentThread() of RemotePendingChanges, the give-back
  comment in LDAPReplicationDomain.replay() - the one line of that file in the PR - and the
  javadoc, assertion messages and one inline comment of RemotePendingChangesTest, whose fixtures
  already had getNextUpdate() called from a thread which cleared nothing (review round 2).
• The pin the redelivery-regression case (Replication: a change whose replay throws is left owned by a thread which is gone #922) gives holds only on the arm where the parent's own
  thread hands the child out: nothing orders the parker's own getNextUpdate() call, made right
  after checkDependencies() parks the child, against the parent being released. Named in that
  case's javadoc rather than forced - measured with a mutant (below). Also fixed that case
  javadoc's self-contradiction, and three more statements of the rule this PR refutes that the
  squash brought in and round 2's rebase missed: RemotePendingChanges'
  releaseParkedChangesOwnedByCurrentThread() (issue Replication: a change parked as a dependency is left owned by a thread which never comes back to it #954's own method) and
  UpdateOperationTest's aChangeParkedByAThreadThePoolStoppedIsDeliveredAgain (Replication: a change parked as a dependency is left owned by a replay thread the pool stopped #986's own case)
  (review round 3).

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
queue arm: the dequeued child sleeps 500 ms ahead of checkDependencies() - original tests	🔴 2/2 - thread 1 vs thread 0, access log with the parent's ADD RES ahead of the child's ADD REQ
same mutant - tests of this PR	🟢 2/2 - the parent is held until the child is seen parked, so the child is handed out rather than replayed from the queue
#923 pin: catch (Throwable) → catch (Exception) in ReplayThread.java:133 - PR head of round 1 (set)	🟢 1/1 - the pin was gone: the assertion named the thread which ran the redelivery
same mutant - this round (compareAndSet)	🔴 1/1 on the isAlive assertion, msgID=140 … Replica replay thread 0 … terminate abnormally … LinkageError in the server log
fail() injected right before parked.release(), whole UpdateOperationTest	🔴 1 of 33, 180 s - the injected one; no cascade, the child is applied cleanly once the finally lets the parent go
no mutant, whole UpdateOperationTest	🟢 33/33, 146 s; RemotePendingChangesTest 🟢 21/21
#922 pin: return in place of the getNextUpdate() call after the ack throws, alone	🔴 3/3 - hangs on the getEntry() wait, killed
same mutant, stacked with a 500 ms delay of the parker's own getNextUpdate() call	🟢 3/3 - the parker takes the child back on a road #922 never runs (review round 3)

The mutants are not part of the PR. The branch sits directly on master at 80481f7: the change,
the round-2 commit and the round-3 commit above, rebased over the squash of #988 (#954 and #986).
That squash rewrote the javadocs round 2 rewrote - the give-back of the parked changes of an
unwound or a stopping thread - and brought seven more statements of the rule this PR refutes
("replayed by whichever thread clears the change it was waiting for") into
LDAPReplicationDomain, ReplayThread, UpdateOperationTest and RemotePendingChangesTest; the
round-2 commit carries them as it carries the ten it had, so that its title holds. Round 3 found
three more which entered the same way and were missed. On this head: UpdateOperationTest 41/41,
247 s; RemotePendingChangesTest 24/24.

[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

[vharseko]

Round 2 is 89bde29, on top of the branch rebased onto master 7243c66 (it sat on 776339a).

issue (blocking) - the #923 assertion named the redelivering thread. Confirmed as described: errorFor() runs the predicate ahead of the budget, thrown stays registered until the finally, so the by-hand redelivery wrote replayingChild last. It is compareAndSet(null, …) now - thrown is registered before the child is delivered and its budget is one, so the first write is the replay the Error unwound. Pinned with your mutant, catch (Throwable) → catch (Exception) in ReplayThread.java:133: round 1 (set) green 1/1, this round red 1/1 on the isAlive assertion, msgID=140 … Replica replay thread 0 … terminate abnormally … LinkageError in the server log.

thought - the cascade on a red exit before the redelivery. Not taken, on a measurement. After processUpdateDone() the message's getCSN() is read only under replayAbandoned / replayFailed (LDAPReplicationDomain.java:3420-3441); the clean road goes straight to getNextUpdate(). The finally runs thrown.deregister() ahead of parked.deregister(), so once the parent is let go the child is applied without a throw and commits inside op.run() - nothing is left at the head of pendingChanges. fail() injected right before parked.release(), whole class: 33 run, 1 failure, 180 s - the injected one, no cascade. What is left is the thrownCount wait going red with the child never replayed at all in 60 s, which is a broken replay rather than a timing, and the class timing out behind it is the right size of signal for that. A cover(childCsn)-guarded redelivery in the finally would make the likelier road worse: red before the hand-out, the child is still queued, the redelivery is refused as a duplicate for the 60 s of its loop, and its assertion replaces the one which went red.

suggestion - the queue arm. Measured with your mutant, Thread.sleep(500) ahead of checkDependencies(op, msg): the original tests red 2/2 - Replica replay thread 1 vs thread 0, "to refer to the same object", access log with the parent's ADD RES ahead of the child's ADD REQ - and the tests of this PR green 2/2, the child parked and handed out. Rows in the Testing table.

suggestion - the rule survives. Rewritten at the three sites named and at seven more which stated it: getChangeOwnedByCurrentThread(), the give-back comment in LDAPReplicationDomain.replay() (the one line of that file in the PR now), and the javadoc, two assertion messages and one inline comment of RemotePendingChangesTest - whose fixtures already had getNextUpdate() called from a thread which cleared nothing.

Whole UpdateOperationTest with the compareAndSet in: 33/33, 146 s. The final tree - comments and javadoc on top of that - RemotePendingChangesTest 21/21 and the two tests of this PR 2/2.

[vharseko]

@maximthomas two more since the table above, both on 2026-09-17, both the same shape:

day	PR under test	leg	first failure, then the cascade
09-17	#985	ubuntu-latest, 17	thread 20 vs 19 at 0.21 s, 16 of 40
09-17	#997	ubuntu-latest, 25	thread 20 vs 19 at 0.21 s, 15 of 39

Twelve legs in six days; #997 is an index-configuration change which comes nowhere near the replay. Both legs were re-run this morning - the third rerun of that class this week.

Round 2 (89bde2964b) answered your blocking item - the #923 liveness assertion is made on the thread which met the Error, whichever one it was - and measured the queue arm with the mutant; the branch is one commit on master, MERGEABLE, 24/24 green since yesterday noon. Could you take another look?

[maximthomas]

praise: The fix for the round-1 issue is the right one and it measures.

• replayingChild.compareAndSet(null, Thread.currentThread()) at opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3340 keeps the thread which met the Error: catch (Throwable t) → catch (Exception t) at opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/ReplayThread.java:133 is now red 1/1 on :3409 at 89bde29 (it was green at bb29004).
• The wait for dependent-changes-size == initial + 1 before parked.release() (:3243, :3358) is what makes "replayed at all" mean "handed out by getNextUpdate()": the child cannot come from the replay queue anymore.
• The hand-out rule at opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java:578-582 now says "as a rule … though the one which parked it … takes it back itself" — that is the rule the code implements.

---

question (non-blocking): theChangesParkedBehindAChangeWhoseAckFailedAreReplayed pins the #922 fix only on the arm where the parent-committing thread is the hand-out.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3243-3251

The wait at :3243 sees +1, which addDependency() publishes before the parker calls getNextUpdate() (LDAPReplicationDomain.java:3448 is the parker's next statement after checkDependencies(), everything in between skipped on dependency == true). Nothing orders that call against parked.release() at :3247. On the usual arm the parker got null and left, only the parent's thread can hand the child out, and a revert of #922 (leaving catch (Throwable ackFailure) without reaching :3448) strands the child — getEntry() times out, red. On the delayed-parker arm the parent commits first, the parker's own getNextUpdate() takes the child back, and :3251 is green under the same revert. The 300 ms parker-delay mutant in the PR body shows the case passes on that arm; nothing shows it pins there. Is that acceptable as is, or do you want it named?

Pin (measurement, not a test change): stack the parker-delay mutant with the #922 revert — expected green; then the javadoc says the pin is per arm, or the case forces the usual arm.

---

nitpick (non-blocking): The case javadoc states an invariant the wait does not give, and contradicts itself three lines later.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3180-3181, :3184-3186

:3180-3181 — "with the parent held until then, the thread which committed the parent is the one left to call it"; :3184-3186 — "a parker which is slow to get there takes the child back itself when the parent commits in between". Both cannot be the rule; the second is the one in the PR title and at RemotePendingChanges.java:580. A reader who takes :3181 as the invariant re-adds the isSameAs(replayingParent) this PR removes. Only occurrence of the unhedged form at HEAD.

   * from anymore: only {@code getNextUpdate()} hands it out, and with the parent held until
   * then, the thread which committed the parent is, as a rule, the one left to call it - a
   * change nobody handed out is replayed by no one at all, and the wait below is what says so.

[vharseko]

Rebased onto master at 80481f756d - the squash of #988, which brought #954 and #986 in - now that the branch was conflicting: the two commits are a67128e08b and e7b7f062de. Your approving round on 89bde2964b came in while this was being rebuilt; nothing of it is taken in this push, it is answered separately. What the rebase had to decide:

The javadocs both rewrote

Five hunks, all prose, all in the javadocs round 2 rewrote - the comment in replay(), changeBeingReplayed, getChangeOwnedByCurrentThread(), addDependency() in RemotePendingChanges, and the case in RemotePendingChangesTest - where #954 had rewritten the same paragraphs for the give-back of the parked changes of an unwound thread. Each carries both now: the rule round 2 states - a parked change is handed out 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 lands before it gets there - and the road #954 added, releaseParkedChangesOwnedByCurrentThread(), which unparks and releases in one step so that the two roads cannot hand a change out at once. The comment in replay() is #954's outright: it says the parked changes are given back, and no longer states the rule.

Seven more statements of the rule

The squash brought the sentence this PR refutes - "replayed by whichever thread clears the change it was waiting for" - into seven places round 2 never saw: two in LDAPReplicationDomain (the last resort of replay() and the javadoc of giveBackParkedChanges()), the give-back comment of ReplayThread.run(), two in UpdateOperationTest (the javadoc of #954's case and a comment in it) and two in RemotePendingChangesTest. Left as they were, the title of round 2 would not hold, so they say what the ten it rewrote say - "whichever thread calls it first once the change it was waiting for is gone - the thread which cleared it, as a rule" - one clause each, folded into the round-2 commit. git grep finds no other occurrence.

Runs

UpdateOperationTest 41/41 - the cases of #954 and #986 included - and RemotePendingChangesTest 24/24, one JVM per class. The mutants of the table are not re-measured on this head. The description says so, in the paragraph on where the branch sits.

[vharseko]

question - the pin holds on one arm. Confirmed and measured: stacked your parker-delay mutant (500 ms after checkDependencies() parks the child) with a revert of #922 (return in place of the getNextUpdate() call after the ack throws) - reverted alone, every run hangs past the getEntry() wait and is killed by the harness (3/3); reverted with the delay, every run is green (3/3), the parker's own getNextUpdate() call takes the child back on the arm the parent's thread never gets a chance to run. Named it in the javadoc rather than forcing the arm - the test has no hook between checkDependencies() parking the child and the parker's own next statement to order against parked.release(), and forcing it would need one.

nitpick - the case javadoc contradicts itself. Fixed - the sentence now reads "is, as a rule, the one left to call it", matching your suggested wording.

Also found three more sites carrying the rule this PR refutes, none in your two rounds' lists: RemotePendingChanges.java (javadoc of releaseParkedChangesOwnedByCurrentThread(), #954's own method) and UpdateOperationTest.java (javadoc of aChangeParkedByAThreadThePoolStoppedIsDeliveredAgain, #986's own case) - all three entered with the squash and were not on the rebase's list of seven. Rewritten the same way as the rest.

Runs: theChangesParkedBehindAChangeWhoseAckFailedAreReplayed 1/1, whole UpdateOperationTest 41/41 (247s), whole RemotePendingChangesTest 24/24.