[vharseko]

Fixes #1195, fixes #1197, fixes #1199, fixes #1200, fixes #1201, fixes #1220, fixes #1221, fixes #1223, closes #1203.

The defect

When a directory server connects to a replication server which lacks some of its changes, LDAPReplicationDomain.RSUpdater republishes them from the historical information of the entries (buildAndPublishMissingChanges()), searching by windows of 10 seconds, while the local changes done meanwhile are only recorded in the domain state (PendingChanges.pushCommittedChanges()) and left to it. Under load it loses changes three ways.

In the run of #1195 the one switch of replication server lost 81 changes of cv-c3, not the two deletes the convergence check saw: 21 adds, 2 deletes and 58 modifications. The sequence numbers of that replica in the changelog have 12 gaps, all of them in the window of that recovery and none elsewhere in the run. Later deletes healed the adds, and a check which compares DNs does not see the modifications.

1. Search against state (67 of the 81). A round chose what to publish by the domain state read after its search (state.cover()). A change committed while the search ran was not in what it found but was covered by that state, so a later change went out in its place and the next round started past it. Deletes escaped only because they are kept aside as they are done.
2. The "no changes" jump (14). A round which found nothing it could publish moved ten seconds ahead, which ended the recovery whatever had been done meanwhile: what the pending changes had left to it was never searched for, and the deletes kept aside were cleared.
3. The hand-over. The pending changes went back to publishing before the broker took them: its recovery flag was cleared only when the thread ended, and a change pushed in between was refused while the domain state recorded it as done.

The replication server does not notice: it takes no change of a replica older than the newest one it holds (LogFile.appendWouldBreakKeyOrdering()), and its server state holds the newest CSN of each replica, so missing-changes reached 0.

A publisher which failed also left the pending changes in recovery, so no later local change of the session was published (#1197), and a round whose search failed part-way still published what it had found, which made the changes it missed undeliverable (#1201).

A session which started while the publisher of the previous one ran got no recovery of its own (#1199): the running publisher went on from where it had got to, on the new session, whose replication server may hold less than the one before and then took none of the older changes it missed; or it had already ended its recovery and left without running the one the new session needed. The list of the deletes kept aside, capped at 10000, took what the searches found as well, and dropped its oldest entry - a change a round had just found, or a delete no replication server had - without a trace (#1200); it also kept a local delete which did not go through, and the deletes the data no longer reflected once it was restored or imported (#1221). Rebuilding the local changes of a recovery from the historical information races the commits, cannot publish a change whose values a later change replaced, and holds the local changes back while it searches (#1203). And for as long as a recovery ran, the broker refused every message which was not a recovery one - the acknowledgements of the changes replayed here, the messages of a total update, the monitoring requests - not only the changes of this replica (#1220). A domain which read back a ServerState with no change of its own - the delete of the base entry takes the saved state with it - had its CSN generator go back to the clock, and numbered its next change as, or before, a change of this replica the replication server missed: one of the two was lost (#1223).

The change

• The local changes done during a recovery are kept (Replication: keep the messages of the local changes done during a recovery, rather than drop them and rebuild them from the historical information #1203). While a recovery runs, the pending changes keep the messages of the local changes they push, in order, rather than drop them. The search of the recovery only covers the changes pushed before the first one it kept: up to a CSN fixed then, which no commit moves - every change up to it has its entry written, so the search sees it. The kept messages go out next, as they were done, a change whose values a later change replaced included. Once a round has caught up with the local changes and some were pushed while it ran, the round after it holds them back (PendingChanges.holdBackLocalChangesWhile(), under the lock the pending changes are pushed under) to publish those, and the recovery ends with it: the local operations wait for that publish alone, and no search runs under the hold. PendingChanges.recoveryUntil() switches the pending changes and the broker back together, under that lock.
• What a recovery keeps stays until it ends: a publisher whose broker refused a change, or whose session was replaced, leaves it to the one which goes on, which may start from further back. Past 10000 changes or 32 MiB, the recovery keeps none and searches for them from where it has got to, as below. So does a recovery whose round, holding the local changes back, does not reach the last local change with what it kept - a change which should have been kept is not there - rather than go round for ever under the lock every local write waits for.
• A kept change is published as not assured, as the changes a recovery rebuilds are. Nothing orders it with the wait of its operation for the acknowledgement: an operation which still waits gets none and times out, as it does when a round publishes the change later. LDAPUpdateMsg.setNotAssured() drops the encoding the message cached.
• The announcement that the replica goes offline is given up on while a recovery runs, as one a change in flight holds back is: it carries the newest CSN of this replica, and the replication server would take the replica for offline from there - its change number index past the changes which reach it afterwards.
• The search. A round publishes up to where the search of the recovery ends, or - for a recovery which kept too many changes - up to the last local change read before its search, or up to the end of its window if that comes first, and the next round starts from there. A round never goes past that bound. A recovery which searches for the local changes ends as before: the rounds after one which caught up hold the local changes back until they reach the last one.
• The end of a window carried the sequence number 0xffffffff: -1 in a CSN, and the greatest in the text the index of ds-sync-hist compares, so a change in the last millisecond of a window was in neither that window nor the next one. The end carries Integer.MAX_VALUE, which reads the same in both, and the search listener takes a change numbered exactly the end, as the search filter does.
• One session per recovery run (Replication: a session which starts while the missing change publisher of the previous one still runs gets no recovery of its own #1199). ReplicationBroker.publishRecovery(msg, session) takes the session the recovery was started for (getSession(), read as the session starts), and refuses the message for any other. It returns false when the broker refuses the message: it reaches no replication server, its session was replaced, or it was stopped. The round then fails, and nothing it had not published is lost: the deletes kept aside and the local changes the recovery keeps stay where they are. A refusal logs a debug line, not ERR_CANNOT_RECOVER_CHANGES.
• A session which starts while the missing change publisher runs hands it its start - the newest change of this replica its replication server holds - and its session, and the publisher goes back there and publishes on that session. The start of a recovery, its end, the hand-over and the exit of the publisher are decided under one lock (recoveryLock), which is taken inside the connect phase lock of the broker and inside the lock of the pending changes, and under which neither is taken: a recovery is not ended while a start is handed over, and the publisher does not leave before it has run from it.
• What the broker refuses during a recovery (Replication: while a recovery runs, the broker refuses every message which is not a recovery one - acknowledgements, total updates, monitoring requests - and not only the changes of the replica #1220). While a recovery is required, the broker refuses the UpdateMsgs only: the changes of this replica, and the announcement that it goes offline. The acknowledgements, the messages of a total update, the monitoring requests and the reset of the generation ID go through.
• The next session ends a recovery a refused publisher left even when its replication server misses no change of this replica: sessionInitiated() then starts a missing change publisher all the same, or hands its start to the one which runs, which finds nothing to publish, or the changes done since, and ends the recovery under the lock of the pending changes. The recovery is not ended in sessionInitiated() itself, which runs under the connect phase lock of the broker: a change being pushed waits for that lock holding the lock of the pending changes.
• A round reads the result of its search before it picks anything: a search which did not go through publishes nothing. A search which finds no base entry went through - the suffix is gone, and the deletes kept aside are all there is - if the backend of the domain is there: a base DN with no backend, which an import has taken away, answers the same, and that search found nothing of what is there.
• A recovery whose search failed, or which threw, does not end: the publisher logs ERR_MISSING_CHANGES_NOT_PUBLISHED, waits with the session up (one second per failure in a row, up to ten), lets go of the recovery, and asks for a session restart, which the state checkpointer runs. The recovery of the next session publishes the changes; a session which starts during the wait hands its start over, which ends the wait, and the publisher runs from it with no restart. A publisher which throws an error - which no catch of an exception takes - has the session started again as well. Ending the recovery instead would let the next local change out over the ones it did not publish, which the replication server would then never take.
• A recovery publishes nothing while a total update replaces the data of this replica: a round refuses, and so does each publish of a round in flight, as what it would publish was done on the data the import replaces. The session the import starts again recovers from the state the new data holds.
• The deletes kept aside (Replication: a full map of the operations kept aside for a recovery drops its oldest one, which can be a change the replication server still lacks #1200, Replication: the deletes kept aside for a recovery outlive a restore or an import of the data, and a later recovery deletes on the other replicas entries this one keeps #1221). What the search of a round finds is the round's own, and the list of deletes kept aside holds deletes only. The deletes a round publishes stay in the list, and so do the ones at or before where it starts, and the list is no longer cleared when a recovery ends: the replication server of a later session may miss them, and they have nothing else to be published from. The cap of 10000 drops the oldest first; one which no replication server has taken logs ERR_UNPUBLISHED_DELETE_DROPPED with its DN and CSN.
• A local delete which did not go through - its conflicts were resolved, and then the access control, a plugin or the backend refused it - is taken out of the deletes kept aside, before the pending changes let the later changes out: a recovery published it as a delete which was done, and the replicas it reached deleted an entry this one keeps.
• Once the ServerState is read from the data again - as the domain is enabled, or after a total update into it - the deletes kept aside and the local changes a recovery kept which the data does not reflect are dropped: the data was restored or imported since. What comes after the newest change of this replica that state holds is not in the data, as the state covers the historical information of the entries - except for a delete, which leaves none, and which the state misses too when it could not be saved: a delete of the base entry takes the saved state with it. A delete is dropped only if its entry is in the data.
• The state read back covers the historical information whether or not the last save wrote it: as it is loaded, PersistentServerState brings it up to the changes of this replica the entries hold past the newest one it read. A state which holds none of them is left as it is - that search would go through every entry the replica ever changed - and such a state comes back once the base entry, which holds the saved state, was deleted and added back, and no save wrote into it since: the add of the base entry was then dropped, and the recovery published the deletes of the suffix alone. loadDataState() brings such a state up from the first change the recovery kept. A domain enabled back on its own data loses none of the changes a recovery kept, unless that search does not go through, which ERR_CANNOT_RECOVER_CHANGES tells.
• The CSN generator does not go back (Replication: a domain which reads back a ServerState with no change of its own moves its CSN generator back, and numbers its next changes as, or before, ones already numbered #1223). CSNGenerator.adjust(null) - loadDataState() passes the newest CSN of this replica in the state it reads, which is null when that state holds none - set the generator to the clock and its sequence number to 0. The session which starts next takes it forward to what its replication server holds only, and the changes that replication server misses are the ones a recovery publishes: the next local change was numbered as one of them, or before, and as a change of another replica it had followed, by as much as that replica's clock runs ahead. An adjust with no CSN leaves the generator where it is.
• The search listener keeps to the changes of this replica, as the search was meant to. The changes of the other replicas found in the same entries were handed over too, as CSNs are ordered by time first; a replication server which had one of them from here first would drop the older changes of that replica reaching it afterwards.
• Test hooks (@VisibleForTesting): setMissingChangesSearchHook() hands the search of each round to a test, which makes a change while a round searches, or fails the search; setMissingChangesPublisherRunHook() runs after each run of the publisher, before it takes a start handed over or leaves; setKeptChangesPublishedHook() runs after a round has published the local changes the recovery kept, before it ends the recovery; setMaxDeletesKeptAside() and PendingChanges.setMaxKeptChanges() lower the caps.

Not fixed here

Found on the way and filed:

• Replication: under load a directory server leaves its replication server for lacking only the changes still on their way to it #1198 - under load a directory server leaves its replication server for lacking only the changes still on their way to it (the trigger of Replication: two deletes a directory server made while switching replication servers under load reached no replication server, and missing-changes still reached 0 #1195); fixed by [#1198] Take the own state of a replication server again after each answer of its peers when it gathers the monitoring data #1205.
• Replication: the broker cannot be stopped while a publish waits for send-window credit, as stop() first needs the lock that publish holds #1202 - ReplicationBroker.stop() needs the lock of the pending changes before it sets the flag a publish waiting for window credit checks; the rounds which hold the local changes back hold that lock longer.
• Replication: the broker cannot be stopped while a publish is blocked writing to a replication server which reads nothing, as the write holds the lock stop() needs #1209 - a write to the replication server which blocks holds the lock of the pending changes, and with it the local operations and the replays of their entries.

Tests

HistoricalCsnOrderingTest: the recovery publishes to a broker which records what it is handed, or runs in the missing change publisher of a real session, and the changes done after it are looked up in the changelog of a real replication server - which, where local changes go on during the recovery, has to hold them and nothing else. The tests of the search rounds start the recovery after the changes they search for, or run with a recovery which keeps no local change, as one which kept too many; three of them run both ways.

• a change done while a round searches is published;
• a round which found nothing does not end the recovery past a change;
• a change done once the recovery has ended is published (the hand-over);
• only the changes of this replica are published;
• a recovery ends while local changes go on, and loses none;
• a change in the last millisecond of a search window is published;
• a recovery ends once it holds the local changes back, however far they ran ahead of one window;
• the changes the broker refused are left to the next recovery;
• a session which needs no recovery ends the one a refused publisher left;
• the broker takes the changes again under the lock they are pushed under;
• a stopped broker refuses a recovery message;
• a round whose search failed publishes nothing;
• the deletes a round threw before publishing are left to the next recovery;
• a recovery whose search failed, and one which threw, have the session started again, and the next session publishes every change, in order, the one done during the failed round included;
• a recovery which threw an error has the session started again;
• a recovery which keeps failing waits longer before each restart, and forgets its failures once it succeeds;
• a session which starts while a failed recovery waits is recovered at once;
• the deletes of a suffix removed during a recovery are published;
• a search which finds no backend fails its round;
• the deletes kept aside outlive a session to a replication server which holds no change of this replica;
• a recovery left on a replica with no change of its own ends;
• a change numbered exactly the end of a search window is published;
• a change whose values a later one replaced is published;
• no search runs while the local changes are held back;
• a recovery which kept too many changes searches for them and loses none;
• a recovery which keeps the local changes ends while they go on and loses none;
• a recovery whose kept changes miss one searches for it;
• the local changes a recovery kept outlive a disable, one whose save did not write the state too, and go with the data they were done on;
• a suffix deleted and added back during a recovery outlives a state which could not be saved into the base entry added back;
• the deletes kept aside go with the data they were done on;
• a broker refuses a recovery message for a session it replaced;
• a broker with a recovery to run refuses the changes only;
• a session which starts while a recovery runs gets every change its replication server misses;
• a session which starts as the publisher leaves gets a recovery of its own;
• a recovery a session which started meanwhile needs is not ended;
• a session which starts while a round holds the local changes back to publish the ones the recovery kept gets them all - it comes up as the broker reconnects, which takes no lock of the pending changes;
• the changes a round finds take no room among the deletes kept aside;
• a delete no replication server took is not dropped without a trace;
• a delete which failed is not published by a recovery;
• the deletes of a suffix outlive a state which could not be saved, the ones a recovery kept as well;
• a kept change is published as not assured;
• the replica is not announced offline while a recovery has changes to publish;
• a recovery publishes nothing while a total update replaces the data;
• a replica which reads back a state with no change of its own numbers its next change after its last one, and CSNGeneratorTest: an adjust with no CSN numbers before neither the CSNs the generator generated nor the ones it received.

On f06769e, with the test hooks only, 17 of the 25 new tests of #1195-#1221 are red, each on the part of the change it covers, and two more need a method that head does not have. Six are green there: the fallback past the cap, the load on a recovery which keeps the local changes, the kept changes dropped on a restore, the deletes of a suffix which outlive a state not saved, a kept change published as not assured - parts that head had no way to get wrong, as it kept nothing and dropped no delete - and the wait of round 3. 33 mutants each undo one part of the change, and all of them turn their tests red - two by going round for ever, until the test JVM runs out of heap. The three tests of #1223 are red with the generator of master, which f06769e has: the add of the base entry gets the CSN of the first delete. Three more mutants come with round 5, each red on its test: without the bring-up of a state read back with no change of this replica (f7fcf27 for that test: the replication server gets the deletes of the suffix only), without the check of a start handed over before a held round keeps no more changes, and - in PersistentServerState - without the bring-up of a state read back to the changes past its newest one. The class: 54 tests, all green; CSNGeneratorTest: 6, all green. org/opends/server/replication/**: not run again locally for this round: the run was cut short as the machine ran out of memory, after other test JVMs on it had taken the ports of its servers; the CI runs it.

[maximthomas]

praise: The three losses are fixed where they happen, and the fix for the hand-over is atomic.

• PendingChanges.recoveryUntil() runs onRecovered under the lock pushCommittedChanges() takes, so no change can be pushed between the end of the recovery and the broker taking changes again (PendingChanges.java:291-299).
• On a refusal the round puts the refused FakeOperation and every one after it back into replayOperations (LDAPReplicationDomain.java:6993-7008), so a delete kept aside is not dropped.
• The window end moves from seqnum 0xffffffff to Integer.MAX_VALUE, which orders the same as a CSN and as ds-sync-hist index text, so the last millisecond of a window belongs to it.

---

issue (blocking): A refused recovery publish leaves PendingChanges recovering, and a next session that starts no recovery publishes no local change.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6993-7008, :6868-6875, :862-868, :6788-6803; opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PendingChanges.java:197-205, :291-299

When publishRecovery() returns false (connectionError, or a broker stopped by disable()/restartService()), the round returns MissingChangesRound(fromCSN, false, false, false), buildAndPublishMissingChanges() returns false before recoveryUntil() runs, and the RSUpdater finally clears only the broker flag. recoveringOldChanges is cleared only by recoveryUntil(); setRecovering() has a single caller, which passes true. The next sessionInitiated() starts no recovery when the RS's CSN for this replica is at or past ours (no local change since the refusal, e.g. back on the RS that had everything before the switch), when the RS holds none or seqnum 0, or when the rsUpdater compareAndSet loses to the old thread (#1199). For that whole session pushCommittedChanges() only records each local change in the ServerState and publishes none of them. On master the refusal was ignored, recoveryUntil() ended the recovery and that session published normally. The description's "left to the recovery of the next session" assumes a recovery that these branches never start. If routing a refusal into the #1197 exit was deliberate, it also needs the pending changes to leave recovery when no recovery follows.

// PendingChanges
  /** Ends a recovery which failed and which no recovery of a later session ends. */
  synchronized void endRecoveryLeftByAFailedOne(Runnable onRecovered)
  {
    if (recoveringOldChanges)
    {
      recoveringOldChanges = false;
      onRecovered.run();
    }
  }

// LDAPReplicationDomain.sessionInitiated()
      boolean recoveryStarted = false;
      if (replServerMaxCSN != null && replServerMaxCSN.getSeqnum() != 0)
      {
        CSN ourMaxCSN = state.getMaxCSN(getServerId());
        if (ourMaxCSN != null
            && !ourMaxCSN.isOlderThanOrEqualTo(replServerMaxCSN))
        {
          pendingChanges.setRecovering(true);
          broker.setRecoveryRequired(true);
          final RSUpdater rsUpdater = new RSUpdater(replServerMaxCSN);
          recoveryStarted = this.rsUpdater.compareAndSet(null, rsUpdater);
          if (recoveryStarted)
          {
            rsUpdater.start();
          }
        }
      }
      if (!recoveryStarted && this.rsUpdater.get() == null)
      {
        pendingChanges.endRecoveryLeftByAFailedOne(() -> broker.setRecoveryRequired(false));
      }

Pin: after a recovery the broker refused (and the broker flag cleared as the RSUpdater finally does), start a session against an RS that holds the domain's last CSN, then replace(dnA, …) must pass assertInTheChangelog. The #1199 race stays, as filed.

---

suggestion (non-blocking): No test pins that the broker flag is cleared under the PendingChanges lock.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7024-7025, opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:455

aChangeDoneOnceTheRecoveryEndedIsPublished fails if onRecovered.run() is deleted, but not if it moves: pass () -> {} and call broker.setRecoveryRequired(false) after if (round.endedRecovery), outside every monitor, and the class stays green. The single-thread case changes only after the call returns, and the two writer cases end on a held round, where the moved clear still runs inside holdBackLocalChangesWhile()'s monitor. The hand-over race of the third bullet could come back unnoticed.

      // in a copy of aChangeDoneOnceTheRecoveryEndedIsPublished, after startRecovery(rd1)
      // (import java.util.concurrent.atomic.AtomicBoolean)
      final Field pendingChangesField = LDAPReplicationDomain.class.getDeclaredField("pendingChanges");
      pendingChangesField.setAccessible(true);
      final Object pendingChanges = pendingChangesField.get(rd1);
      final Field brokerField = ReplicationDomain.class.getDeclaredField("broker");
      brokerField.setAccessible(true);
      final ReplicationBroker realBroker = (ReplicationBroker) brokerField.get(rd1);
      final AtomicBoolean clearedUnderTheLock = new AtomicBoolean();
      brokerField.set(rd1, new TestBroker(new ArrayList<ReplicationMsg>())
      {
        @Override
        public void setRecoveryRequired(boolean required)
        {
          clearedUnderTheLock.set(!required && Thread.holdsLock(pendingChanges));
        }
      });
      try
      {
        assertTrue(rd1.buildAndPublishMissingChanges(rsMaxCSN, new TestBroker(new ArrayList<ReplicationMsg>())));
        assertTrue(clearedUnderTheLock.get(), "the broker takes changes again under the lock they are pushed under");
      }
      finally
      {
        brokerField.set(rd1, realBroker);
        realBroker.setRecoveryRequired(false);
      }

Pin: the case above turns red on the moved clear, because the round that ends the recovery here is not a held one.

---

suggestion (non-blocking): The refusal road is tested only against the TestBroker stub; the real publishRecovery() result never runs.

opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java:2283-2286, opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:117

Every case passes a TestBroker whose publishRecovery() returns its writes flag, so return publish(msg, true, true) is never executed: the mutant publish(msg, true, true); return true; (master's dropped result) survives the class. changesTheBrokerRefusedAreLeftToTheNextRecovery pins the domain-side put-back only.

  @Test
  public void aStoppedBrokerRefusesARecoveryMessage() throws Exception
  {
    final DN baseDN = DN.valueOf(TEST_ROOT_DN_STRING);
    TestCaseUtils.initializeTestBackend(true);
    ReplicationServer rs = createReplicationServer();
    ReplicationBroker broker = null;
    try
    {
      broker = openReplicationSession(baseDN, OTHER_SERVER_ID, 100, rsPort, 1000);
      broker.stop();
      assertFalse(broker.publishRecovery(new WindowProbeMsg()), "a stopped broker writes nothing");
    }
    finally
    {
      stop(broker);
      remove(rs);
    }
  }

Pin: publish()'s loop is skipped once shutdown is set, so the case is red on the mutant and green at the head.

---

issue (non-blocking): stop()/disable() waits on the PendingChanges monitor for every held-back round, without a bound while the RS withholds window credit.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6868-6871, opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java:2737-2741

holdBackLocalChangesWhile() holds the monitor across the round's search and every publishRecovery(). ReplicationBroker.stop() calls the synchronized PendingChanges.putReplicaOfflineMsg() before shutdown = true, and publish()'s loop ends only on done, connectionError or that flag. A disable() during a held round therefore waits, holding serviceStateLock and sessionLock, and local writes wait with it. Filed as #1202, recorded here only so the follow-up keeps the disable() road in view.

---

issue (non-blocking): aChangeInTheLastMillisecondOfASearchWindowIsPublished fails on correct code when more than ~10 s pass between addPerson("b") and replace().

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:589-604

endOfFirstWindow is fixed right after addPerson("b"). Then the replication session opens (1 s timeout) and waitUntilReplayed() runs (10 s budget) before replace(). CSNGenerator takes the wall clock once it passes the adjusted lastTime, so on a stalled runner the change is numbered past the window and the assertEquals fails.

      other = openReplicationSession(baseDN, OTHER_SERVER_ID, 100, rsPort, 1000);
      final DN dnA = addPerson("a");
      final DN dnB = addPerson("b");
      final CSN rsMaxCSN = rd1.getLastLocalChange();
      final long endOfFirstWindow = rsMaxCSN.getTime() + SEARCH_WINDOW_MS;

---

suggestion (non-blocking): publishRecovery() changes its descriptor from ()V to ()Z.

opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java:2283

Code compiled against an earlier release that calls it gets NoSuchMethodError. In this repository the only callers are LDAPReplicationDomain:6993 and the test override. If ReplicationBroker is not meant as an extension point, nothing needs to change; otherwise a line in the release notes covers it.

---

nitpick (non-blocking): The publishRecovery() @return and the buildAndPublishMissingChanges() javadoc promise more than publish() returns.

opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java:2279-2281

publish() calls the void Session.publish(msg), which drops Session.publish(msg, null)'s false for a closing session or an unencodable message, and returns true. The javadoc covers only connectionError and a stopped broker.

   * @return {@code false} if the broker refused the message: no replication server was reachable,
   *         or the broker was stopped; {@code true} once a session accepted it

---

nitpick (non-blocking): assertEachChangePublishedOnce does not check "once".

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:750-768

The helper checks that the recovered CSNs are an ordered prefix of done and that the rest are in the changelog. It never checks that a recovered CSN was not also published by the broker. Rename it assertEachChangePublished, or add that check.

[vharseko]

Round 2: 827f8b2. The description is updated.

issue (blocking): a refused recovery publish leaves PendingChanges recovering. Confirmed, and fixed - but not with the suggested code, which would deadlock.

sessionInitiated() runs inside synchronized (connectPhaseLock): ReplicationBroker.connectAsDataServer() :764 → connectToReplicationServer() :823 → domain.sessionInitiated() :959. A local change takes the two locks the other way round: PendingChanges.commitAndPushCommittedChanges() holds the monitor of the pending changes, and ReplicationDomain.publish() → ReplicationBroker.publish() takes connectPhaseLock to read the session (:2334). A synchronized endRecoveryLeftByAFailedOne() called from sessionInitiated() - on every session start which starts no recovery, as it reads the flag only once it holds the monitor - deadlocks a reconnect with a local change pushed during it, and every later write of the domain queues behind them. The code there already avoids that monitor: setRecovering() is not synchronized.

The snippet also keys on recoveryStarted rather than on whether a recovery is needed. When the CAS loses to the old thread (#1199) and that thread deregisters before this.rsUpdater.get(), it ends a recovery the replication server needs; the next local change goes out, and the replication server can no longer take the older ones it misses.

So the session starts a publisher instead. When its replication server misses no change of this replica but pendingChanges.isRecovering() (the field is volatile now), sessionInitiated() starts an RSUpdater from the CSN the replication server holds of this replica - from our own when it holds none - and touches neither flag (LDAPReplicationDomain.java:6801-6817). The publisher finds nothing to publish, or the changes done since the check, and ends the recovery through recoveryUntil(), under the monitor, in its own thread. If the publisher of the previous session still runs, the CAS loses and that one ends it; if it fails, that is #1199 as filed.

aSessionWhichNeedsNoRecoveryEndsTheOneARefusedPublisherLeft is your pin: a refused recovery, the broker flag cleared as the RSUpdater finally does, then disable()/enable() against a replication server which holds the last CSN, then replace(dnA, …) must reach the changelog. broker.start() connects before enable() returns, and the test asserts that the session is up before the change: a change done earlier would put the replica ahead of the replication server, and the session would recover it on the old code too. It is red on cfc1f20d5f (… is in the changelog of the replication server expected [true] but found [false]) and on the mutant which drops the new branch, and green at the head.

#1197 keeps the rest: a failed publisher still leaves the recovery on for the rest of its own session.

suggestion: no test pins that the broker flag is cleared under the PendingChanges lock. Added as theBrokerTakesChangesAgainUnderTheLockTheyArePushedUnder, as you wrote it. Your mutant (() -> {}, and the clear after if (round.endedRecovery)) is red on it and green on the rest of the class. While the stub is in place, the listener thread of the domain may call its receive(…) - package-private, so the test cannot override it - and end as on a shutdown; that touches neither assertion, and the domain is deleted at the end of the test.

suggestion: the real publishRecovery() result never runs. Added as aStoppedBrokerRefusesARecoveryMessage; red on publish(msg, true, true); return true;.

issue: stop()/disable() waits on the PendingChanges monitor for every held-back round. #1202 has that road already: the second point of its Effect section says a disable or a restart does not set the shutdown flag of the domain, so stop() waits for the end of the held rounds even with the window open. Nothing changed here.

issue: aChangeInTheLastMillisecondOfASearchWindowIsPublished on a stalled runner. The session of the other replica is opened first now, so only the publish of its change and the replay stay between the read of the last change and replace().

suggestion: publishRecovery() changes its descriptor. Not changed: ReplicationBroker is an internal class of opendj-server-legacy, not an extension point, and its only callers are the domain and the test.

nitpick: the @return of publishRecovery(). Reworded as you suggested, and it says that a closing session still drops the message, as it does any message the broker publishes. The javadoc of buildAndPublishMissingChanges() and of MissingChangesRound.succeeded now speak of a refusal too. publish() does not take the result of Session.publish(msg, null): for a message which cannot be encoded for the peer that is false on every try, and the loop would never end.

nitpick: assertEachChangePublishedOnce does not check "once". It does now. The changelog of the replication server must hold, after the last CSN it had before the recovery, exactly the changes published after the recovery. A change of the recovery which the broker published as well would reach it before those, on the same session, so the two would never match. A mutant which publishes each recovered change through the broker of the domain as well is red on both load tests, and green with the old helper.

Tests: HistoricalCsnOrderingTest 14/14 green; the four mutants above are each red on their test. org/opends/server/replication/**: 3843 tests, 0 errors, 0 skipped, 1 failure - ProtocolWindowTest.saturateQueueAndRestart (missing-changes after a fixed 2-second sleep, :232), which fails the same way on master: 1 run in 5 on master, on cfc1f20d5f and at the head.

[maximthomas]

praise: The round-1 blocker is answered where it arises, and two round-1 nits are fixed.

• The else if (pendingChanges.isRecovering()) arm in sessionInitiated() (LDAPReplicationDomain.java:6801-6816) starts a publisher when the replication server misses nothing, so a refused publish no longer leaves the next session publishing nothing; aSessionWhichNeedsNoRecoveryEndsTheOneARefusedPublisherLeft drives it, and HistoricalCsnOrderingTest ran green here at 827f8b2 (14/14).
• aChangeInTheLastMillisecondOfASearchWindowIsPublished opens its session before the changes (HistoricalCsnOrderingTest.java:640), so only the replay wait sits inside the window, and the precondition has its own message.
• ReplicationBroker.publishRecovery()'s @return and the buildAndPublishMissingChanges() javadoc now say exactly when a change counts as refused.

---

issue (blocking): A recovery round whose search fails on a session that stays up leaves PendingChanges recovering with no publisher.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7050-7052, :6898-6900, :831-867

When the domain's base entry is missing, the range search returns NO_SUCH_OBJECT without an exception (EntryContainer.java:1186, LocalBackendSearchOperation.java:149); succeeded is false, so recoveryUntil() never runs, buildAndPublishMissingChanges() returns false and the RSUpdater finally clears only the broker flag. No next session comes, so every local change is only recorded in the ServerState until the connection drops. On master recoveryUntil() ran before the SUCCESS test with currentStartCSN past the last local change, so the recovery ended and only that window's changes were lost, which is what the unchanged comment in RSUpdater.run() ("This server will start accepting again new updates") still says. A failed window that is not the last, and a search that throws, leave the same state (on master too); the change below covers both. If a failed search was meant to leave the recovery to "the next session": a session that stays up starts none.

// PendingChanges
/** Ends a recovery which cannot go on, so that the local changes are published again. */
synchronized void endRecovery(Runnable onRecovered)
{
  if (recoveringOldChanges)
  {
    recoveringOldChanges = false;
    onRecovered.run();
  }
}

// LDAPReplicationDomain.publishMissingChanges(), where succeeded is computed
final boolean succeeded = op.getResultCode().equals(ResultCode.SUCCESS);
if (!succeeded)
{
  // No next session follows a failed search: what it did not find is left to the repair tool.
  pendingChanges.endRecovery(() -> broker.setRecoveryRequired(false));
}

// RSUpdater.run(), in the catch of an exception out of the search, after the logger.error
pendingChanges.endRecovery(() -> broker.setRecoveryRequired(false));

Left as they are: the refusal (:7036), which the reconnect and the new arm end, and the shutdown exits (:6875, :6891, round == null).
Pin: delete the domain's base entry during a recovery so its round's search returns NO_SUCH_OBJECT, then re-add it and assert the broker receives the add — red at 827f8b2.

---

issue (blocking): On a replication server that holds no change of this replica, the new arm's publisher drops the kept-aside deletes, so a later recovery never publishes them.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6815-6816, :6869-6884, :6993-7010, :835-837, :7027-7033

A publisher fails and leaves the pending changes recovering; a local delete c4 done then is kept aside (:1950) and only recorded in the state. The next session reaches an RS that holds none of this replica's changes: the arm starts the publisher at ourMaxCSN = c4, buildAndPublishMissingChanges() trims every replayOperations key at or below it — c4 and any refused delete put back at :7027-7033 — ends the recovery, and run() clears the map. A later session to an RS whose newest change of this replica is older than c4 cannot find the deleted entry and has no kept-aside delete left: c4 is never published and the other replicas keep the entry. On master no publisher starts on the empty RS, the delete stays kept aside and goes out in that later recovery. Adds and modifies are not hit: that recovery's search finds them in ds-sync-hist.

Fix: for the !replServerHoldsChanges start, leave replayOperations as it is — no trim at :6869-6884, no entry at or below the start CSN taken by the selection at :6996-7009 (skip it without removing it), no clear at :835-837 — and end the recovery as now; the put-back comment at :7022-7026 then holds. Skipping only the trim is not enough: the selection has no lower bound and would send those deletes to the empty RS in the first round.
Or: skip only the trim and let that first round publish them — nothing is lost, but up to 10000 deletes the other replicas already have are re-sent, against the "do not update an empty RS" comment at :6786-6791.
Pin: a refused publisher, a local delete, a session to an RS holding none of this replica's changes (it receives nothing older than ourMaxCSN), then a session to an RS that lacks the delete — the delete is published.

---

suggestion (non-blocking): Nothing pins putting back the first change the broker refuses.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7029, opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:741-760

In changesTheBrokerRefusedAreLeftToTheNextRecovery the first refused change is the modify of dnA, which the second recovery's search finds again; only the delete of dnB needs a put-back, and the loop after :7029 does that. With replayOperations.put(opToSend.getCSN(), opToSend); deleted, the class ran green here: 14/14.
Pin: swap the two changes at :757-760, so the delete of dnB is the first one refused — red under that mutant.

---

suggestion (non-blocking): The new arm's ourMaxCSN and new CSN(now, 0, id) starts are run by no test.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6815-6816, opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:782-816

aSessionWhichNeedsNoRecoveryEndsTheOneARefusedPublisherLeft asserts b in the changelog first, so only replServerMaxCSN is taken; collapsing the ternary to it stays green and, on an RS holding none of this replica's changes, passes null, which ends in an NPE at :6980 and the recovery left set.
Pin: the second blocking issue's test runs the ourMaxCSN start; a variant whose domain state holds no CSN of this replica runs the third.

---

suggestion (non-blocking): No test runs a recovery round whose search is not SUCCESS.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7050-7052

Whatever the first blocking issue settles, nothing turns red when that outcome flips.
Pin: that issue's test.

---

question (non-blocking): Does #1202 also cover replays waiting on the entry locks of local operations parked in a held round?

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6896-6897, opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PendingChanges.java:125

A local modify takes the entry write lock (LocalBackendModifyOperation.java:391) and parks in putLocalOperation(); a replay of the same entry gets BUSY after the lock manager's 9 s, ten times, then takes the BUSY give-up branch (LDAPReplicationDomain.java:3346) and goes down the replay-failure road; a parked delete blocks the whole subtree. A held round lasts as long as its publish, unbounded while the RS withholds window credit or the session reconnects. Minor as long as a held round cannot outlast ~90 s otherwise; bounding the held publish fixes both waits.

---

issue (non-blocking): The new arm's isRecovering() check and the publisher's compareAndSet race the previous publisher's finally.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6801-6833, :864-868

If the previous publisher ends the recovery between the check and the compareAndSet, a second one starts and republishes changes already out (the changelog drops them, the cookie notification fires again). If it is failing and has not reached its finally, the compareAndSet loses and the recovery stays set for the whole session, which the comment at :6810-6812 says cannot happen. The window is microseconds against the broker's 500 ms reconnect sleep. Correct the comment; to close it, re-run the arm's check from RSUpdater.run()'s finally after its compareAndSet(this, null).

---

suggestion (non-blocking): A refused publish logs ERR_CANNOT_RECOVER_CHANGES, whose text blames a search of old changes.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7036, opendj-server-legacy/src/messages/org/opends/messages/replication.properties:65

Every RS loss during a recovery now logs "Error when searching old changes from the database", and the comment adds that the repair tool must re-synchronize, for changes the next session is built to publish; on master the refusal logged nothing. Give the refusal its own message and keep this one for a failed or throwing search.

---

nitpick (non-blocking): The window end new CSN(time + 10000, Integer.MAX_VALUE, id) is a CSN this replica can generate.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:6980

The seqnum is not reset per millisecond and reaches Integer.MAX_VALUE before it wraps, so a change numbered exactly the window end is outside this window's isOlderThan(endCSN); whether the next window then misses it was not traced. Practically unreachable; a comment would do.

---

nitpick (non-blocking): replServers is never cleared, so each later replication server and domain of the class is configured with every earlier case's dead port.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java:94

The refused loopback connects only delay each connect. Clear it in the class's @AfterMethod, or drop the port in remove(rs).

[vharseko]

Round 3: f06769e. The branch is rebased onto master, which has #1205 now; it touches none of these files, and the two earlier commits are unchanged. The description is updated, and the PR now closes #1197 and #1201 as well.

issue (blocking): a recovery round whose search fails on a session which stays up leaves PendingChanges recovering with no publisher. Confirmed, and fixed - but without ending the recovery. endRecovery() lets the next local change out over the changes the recovery did not publish, and the replication server then takes none of them (LogFile.appendWouldBreakKeyOrdering()). Those are the changes of the failed window and the ones after it, plus every local change done during the recovery, which only the domain state records: all of them would be left to the repair tool. Instead, the publisher has the session started again, and the recovery of the next session publishes them.

• publishMissingChanges() reads the result of the search before it picks anything (LDAPReplicationDomain.java:7034-7045). A search which ends with neither SUCCESS nor NO_SUCH_OBJECT throws MissingChangesSearchFailedException, having published nothing and taken nothing out of replayOperations. That is Replication: a missing change publisher whose search fails part-way still publishes what it found, and the changes it missed can never reach the replication server #1201. A round which throws while publishing puts back what it had not published (:7103-7109).

• On any exception out of the rounds - a failed search, or anything thrown - RSUpdater.run() (:859-902) does four things:

  1. logs ERR_MISSING_CHANGES_NOT_PUBLISHED with the cause, which names the CSN of the window for a failed search, and the wait (ordinal 333; 332 is taken by [#1207] Route total updates to a replica that has the server ID of a replication server, report such a replica, and draw the IDs of new replication servers and domains from one set #1211);
  2. waits with the session up: 1 s per failure in a row, up to 10 s. The count is its own, since a replayed change resets the replay one;
  3. in finally, after compareAndSet(this, null), asks for a session restart (sessionRestarts.request(NOW));
  4. leaves the restart to the state checkpointer, which runs it within a second, or once a total update is over.

  restartSession() with the backoff is not used: it stops the session before it waits, and would keep it down for the whole wait. The publisher waits registered, so that a session which starts meanwhile starts no publisher the restart would then cut. Nothing is asked for when the domain is shut down or disabled.

• The next session starts the recovery from what its replication server holds. Usually that replication server is behind by every change left, so the first arm starts it; if it is not, the arm of round 2 does.

• NO_SUCH_OBJECT counts as a search which found everything there is. The base entry is gone, and nothing below it can hold a change, so the deletes which removed the suffix go out and the recovery ends. Otherwise those deletes would wait for the suffix to come back. The search for the generation ID reads that result the same way (:1130-1131). Your pin is theDeletesOfASuffixRemovedDuringARecoveryArePublished:

  • an entry and the base entry are deleted during a recovery;
  • the round's search returns NO_SUCH_OBJECT;
  • both deletes are published and the recovery ends;
  • the base entry added back reaches the changelog.

  It is red on 827f8b2 (a recovery whose search found no base entry is over expected [true] but found [false]).

• What is left: a search which keeps failing stops the outgoing replication of this replica, without loss, until it is repaired, with an ERR and a session restart every 10 s. The other loss path is the cap of 10000 deletes kept aside (Replication: a full map of the operations kept aside for a recovery drops its oldest one, which can be a change the replication server still lacks #1200).

• aRecoveryWhoseSearchFailedHasTheSessionStartedAgain and aRecoveryWhichThrewHasTheSessionStartedAgain drive the real RSUpdater:

  • a session starts with its replication server behind;
  • the first round fails, by its result code or by a throw;
  • the changelog must end up holding exactly the missed changes, in order;
  • the recovery must end, and a change done after it must be published.

  Both are red on 827f8b2: the recovery stays on, and on the throw nothing is published either. aRoundWhoseSearchFailedPublishesNothing pins Replication: a missing change publisher whose search fails part-way still publishes what it found, and the changes it missed can never reach the replication server #1201: on 827f8b2 the failed round publishes what it found. theDeletesARoundThrewBeforePublishingAreLeftToTheNextRecovery pins the put-back. The search hook now takes the search (Consumer<InternalSearchOperation>), so that a test can set its result code; the new tests ran on 827f8b2 with that hook alone.

issue (blocking): on a replication server which holds no change of this replica, the new arm's publisher drops the kept-aside deletes. Confirmed. Fixed wider than you asked: for every start, not only the !replServerHoldsChanges one.

• buildAndPublishMissingChanges() no longer trims, and RSUpdater.run() no longer clears.
• The selection of a round (:7047-7075) skips what is at or before fromCSN without removing it.

The end of any recovery had the same hole: the clear dropped deletes which the replication server of a later session could still miss. The map stays bounded by the cap of 10000 (:1977-1980), which drops the oldest first.

theDeletesKeptAsideOutliveASessionToAReplicationServerWithNoChangeOfThisReplica is your pin:

• the state a refused publisher leaves, then a local delete;
• a configuration change moves the session to a second replication server, which holds no change of this replica and knows of no other. Its publisher starts at ourMaxCSN, ends the recovery, and sends it nothing;
• the session goes back to the first replication server, which lacks the delete;
• the delete reaches its changelog.

It is red on 827f8b2, and on each mutant which brings back the trim, brings back the clear, or removes what the selection skips.

suggestion: nothing pins putting back the first change the broker refuses. Swapped: the delete of dnB is refused first now. It is red on the mutant which leaves the first refused change out of the put-back.

suggestion: the new arm's ourMaxCSN and new CSN(now, 0, id) starts are run by no test. theDeletesKeptAside… runs the ourMaxCSN start. aRecoveryLeftOnAReplicaWithNoChangeOfItsOwnEnds runs the now one: a domain whose state holds no CSN of its own, on an empty replication server. Collapsing the ternary to replServerMaxCSN is red on both: the null start throws, and that publisher now keeps restarting the session instead of quietly leaving the recovery on. Replacing ourMaxCSN with the current time stays green. On an empty replication server both starts publish nothing, and only a change in flight during the session start tells them apart.

suggestion: no test runs a recovery round whose search is not SUCCESS. The tests of the first blocking issue.

question: does #1202 also cover replays waiting on the entry locks of local operations parked in a held round? No: #1202 and #1210 bound stop() only. Master has the same chain for every push under the monitor of the pending changes; a held round only lasts longer. The replay side is now written into #1209, in Code and in Effect: the entry lock, or the subtree lock for a delete or a rename, is held up to putLocalOperation(); the replay gives up after 10 × 9 s and takes the replay-failure road; the session restart which is to deliver the change again waits for the same monitor in stop().

issue (non-blocking): the new arm's check and the compare-and-set race the previous publisher's finally. The comment is corrected (:6859-6862). A publisher whose search failed now keeps the recovery and has the session started again itself, so it no longer leaves the recovery to nobody. The race which remains is with a publisher on its way out after a refusal: it can make this start lose the compare-and-set, and the recovery then waits for the next session. Re-running the check from the finally would need the start CSN, which only a session start reads from its replication server. That hand-over between a publisher and a session is #1199, and it stays there.

suggestion (non-blocking): a refused publish logs ERR_CANNOT_RECOVER_CHANGES. A refusal logs a debug line only now, as master logged nothing: the next session publishes those changes. A failed search or a throw logs the new ERR_MISSING_CHANGES_NOT_PUBLISHED, which says what failed and when the session is started again. ERR_CANNOT_RECOVER_CHANGES stays for PersistentServerState.

nitpick: the window end is a CSN this replica can generate. Traced: the change was lost. The search filter takes the end, but the listener took only what is isOlderThan(endCSN), and the next window starts with isNewerThan(endCSN). The listener now takes the end (:251). In aChangeNumberedTheEndOfASearchWindowIsPublished, the generator is set so that the next local change is numbered exactly the end; the test is red on 827f8b2.

nitpick: replServers is never cleared. It is cleared in an @AfterMethod.

Tests: HistoricalCsnOrderingTest 22/22 green. On 827f8b2 (with the new hook only), 7 of the 8 new tests are red; the eighth, aRecoveryLeftOnAReplicaWithNoChangeOfItsOwnEnds, pins the now start against the mutant above. 10 of 11 mutants are red on their tests: each one undoes one part of this round, and the survivor is the now-for-ourMaxCSN one. org/opends/server/replication/**: 3851 tests, 0 failures, 0 errors, 0 skipped.

[maximthomas]

praise: Both round-2 blockers are fixed where they arise, and the new put-back is pinned.

• RSUpdater.run()'s catch now has a failed recovery request SessionRestart.NOW after its wait (LDAPReplicationDomain.java:859-899). A failed search on a session that stays up no longer leaves PendingChanges recovering with no publisher. aRecoveryWhoseSearchFailedHasTheSessionStartedAgain and aRecoveryWhichThrewHasTheSessionStartedAgain drive both exits.
• The putBackUnpublished() in the catch (RuntimeException e) around the publish loop (:7107) is pinned: deleting it turns theDeletesARoundThrewBeforePublishingAreLeftToTheNextRecovery red (measured here: 1 of 22 cases red). HistoricalCsnOrderingTest runs green at f06769e (22/22).

---

issue (blocking): A local delete that fails in the backend now stays among the deletes kept aside, and a later recovery publishes it.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2518-2524, :1974-1983, :7054-7072

handleConflictResolution() puts a FakeDelOperation into replayOperations for every local delete, before the backend runs it (:1974-1983). If the delete then fails (NOT_ALLOWED_ON_NONLEAF, a pre-operation plugin, the sub-backend check), the failure branch of synchronize(PostOperationOperation) removes it from pendingChanges only. This commit removes the trim at the start of a round and the clear after a successful recovery. Without them the entry stays until a round whose window covers it publishes it (:7056-7071, no other check), or until the 10000 cap evicts it.

Road: the delete fails at csn_f. A later recovery against a replication server that holds later changes of this replica starts past csn_f and skips it. Then a session on a lagging replication server, whose max for this replica is before csn_f, publishes DeleteMsg(csn_f). The replicas fed through that server delete an entry that still exists here, and rename its children as conflicts. Before this commit, the first recovery that started past csn_f trimmed it. Read, not run.

    else if (!op.isSynchronizationOperation() && curCSN != null)
    {
      // Remove an unsuccessful non-replication operation from the pending
      // changes list.
      pendingChanges.remove(curCSN);
      pendingChanges.pushCommittedChanges();
      // A delete which failed must not stay among the deletes kept aside: a later recovery would
      // publish it.
      synchronized (replayOperations)
      {
        replayOperations.remove(curCSN);
      }
    }

Only a local delete puts its own CSN into replayOperations, so the remove does nothing for the other operation types. No lock nests: the pendingChanges calls return before replayOperations is taken. The fix leaves one window open, and it is older than this PR: a round that takes the delete between the put and the failure still publishes it, or puts it back after the remove (:7129-7133). Not checked: whether a cancelled delete skips the post-operation call.

Pin: a HistoricalCsnOrderingTest case that deletes a non-leaf entry (NOT_ALLOWED_ON_NONLEAF), makes a later local change, runs buildAndPublishMissingChanges() from a CSN before the failed delete, and asserts that no DeleteMsg for that DN is published. It is red at this head without the remove.

---

issue (non-blocking): While a failed recovery waits to start the session again, the broker drops every message that is not a recovery one.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:877-888, opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java:2346

The catch sleeps in waitBeforeSessionRestart() (1-10 s, longer with each consecutive failure). Only after that does the finally call broker.setRecoveryRequired(false), and while the flag is up ReplicationBroker.publish() returns false for any non-recovery message. During the wait these messages are lost:

• SAFE_READ acks for the updates replayed here, so their originators time out;
• the ReplicaOfflineMsg of a disable();
• the messages of an export from this replica, so the export fails;
• a ResetGenerationIdMsg.

If the failure persists, the flag is up about 10 s of every 11-12 s. Before this commit the finally cleared the flag right after the failed round. Local updates are not affected: the recovering PendingChanges keeps them. Read, not run.

          // The broker drops every message which is not a recovery one while the flag is up.
          broker.setRecoveryRequired(false);
          waitBeforeSessionRestart(failures, sessionRestartBackoffWakes());

The clear in the finally stays, for the exits that do not wait. Not checked: a session that comes during the wait sets the flag again (:6842) and starts no publisher of its own, so from that reconnect to the finally the flag is up again.

---

issue (non-blocking): A round counts the NO_SUCH_OBJECT from a backend that a failed import left deregistered as a search that went through, so the changes of that window are never published.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7042-7045

LocalBackendWorkflowElement answers NO_SUCH_OBJECT for a base DN that has no backend, as it does for a missing base entry. Take a total update into this replica that fails after preBackendImport() deregistered the backend, for example the lock refusal at :5999-6006. The domain stays enabled and the recovery keeps running. A window searched in that state publishes none of its changes, and the round moves on. The recovery ends at a later window, and every later session finds the replication server at or past this replica's max, so those changes never leave this replica. Before this commit the round stopped on the non-SUCCESS result, and the next session searched that window again. This needs three rare conditions at once. Read, not run.

    final ResultCode resultCode = op.getResultCode();
    if (!resultCode.equals(ResultCode.SUCCESS)
        && !(resultCode.equals(ResultCode.NO_SUCH_OBJECT) && isBaseEntryGone()))
    {
      throw new MissingChangesSearchFailedException(fromCSN, op);
    }

  /** Whether the backend of the domain is there and holds no base entry: its search then went through. */
  private boolean isBaseEntryGone()
  {
    final LocalBackend<?> backend = getBackend();
    try
    {
      return backend != null && !backend.entryExists(getBaseDN());
    }
    catch (DirectoryException e)
    {
      return false;
    }
  }

Still open with this fix: a backend that comes back between the search and the check.

---

suggestion (non-blocking): No test pins the wait before a failed recovery starts the session again, or the reset of its failure counter.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:853, :877-882

Only aRecoveryWhoseSearchFailedHasTheSessionStartedAgain reaches the catch, on a fresh domain that fails once. Two deletions leave it green (read, not run):

• deleting waitBeforeSessionRestart(...) only brings the restart about 1 s earlier, well inside the test's 30 s poll;
• deleting consecutiveRecoveryFailures.set(0) changes nothing on a domain that fails only once.

A tight restart loop, or a wait stuck at the 10 s cap, would pass.

Pin: a case that fails the search twice in a row on one domain and then lets it go through. It asserts that the wait grows on the second failure (1000 → 2000 ms, read from the ERR_MISSING_CHANGES_NOT_PUBLISHED log argument or from a package-private getter of consecutiveRecoveryFailures), and that the count is back to 0 after the success.

[vharseko]

Round 4: f7fcf27. The branch is rebased onto master (8fc60b9, whose changes since round 3 touch the workflows and grizzly only), and the three earlier commits are unchanged. Besides your round 3, this round takes in #1199, #1200 and #1203, two issues filed from your round 3, #1220 and #1221, and #1223, found on the way; the description is rewritten for them, and the PR closes all six as well.

issue (blocking): a local delete that fails stays among the deletes kept aside, and a later recovery publishes it. Confirmed, and not only for a backend which refuses it: handleConflictResolution() runs before the access control, the pre-operation plugins, the sub-backend check, the no-op control and the backend, and any of them can stop the delete. Fixed in the failure branch of synchronize(), as you suggested, but with the remove placed before pendingChanges.remove(), not after (LDAPReplicationDomain.java:2700-2718). The order closes the window you left open:

• a failed delete sits uncommitted in the pending changes until that branch, and every later local change waits behind it, so the domain state - which bounds every round - cannot reach its CSN before it is taken out of the list;
• a round which put it back after the remove: there is no put-back any more, as rounds no longer take deletes out of the list (see Replication: a full map of the operations kept aside for a recovery drops its oldest one, which can be a change the replication server still lacks #1200 below).

A cancelled delete goes the same way: checkIfCanceled() (LocalBackendDeleteOperation.java:258) is inside the try whose finally runs processSynchPostOperationPlugins(), so synchronize() sees a result which is not SUCCESS. Your pin is aDeleteWhichFailedIsNotPublishedByARecovery (NOT_ALLOWED_ON_NONLEAF, a later local change, a recovery from before the delete): red on f06769e, and on the mutant which drops the remove.

issue (non-blocking): while a failed recovery waits, the broker drops every message that is not a recovery one. Confirmed, and not only during the wait: the check at ReplicationBroker.java:2346 refused every message which was not a recovery one for as long as any recovery was required, on master too - the acknowledgements, a total update from or into this replica, the monitoring requests, the reset of the generation ID. Filed as #1220, and fixed at its root: while a recovery is required the broker refuses the UpdateMsgs only (:2361-2375).

Your snippet is not taken, as it would let nothing more through. The two UpdateMsgs a recovery leaves to the broker are the local changes, which the pending changes keep while a recovery runs, and the announcement that the replica goes offline. That announcement must not go out while a recovery has changes to publish, whatever the broker flag says. It carries the newest CSN of this replica, and the replication server takes the replica for offline from there: notifyReplicaOffline() writes no record into the log of the replica, but the change number indexer moves past the changes which reach it afterwards. The flag did not keep it back in every case: a recovery left by a refused publisher, the exit of the publisher before the session restart it asks for, and the broker.stop() of that restart. The pending changes now give it up for as long as a recovery runs, as they give up one a change in flight holds back (PendingChanges.java:234-247). theReplicaIsNotAnnouncedOfflineWhileARecoveryHasChangesToPublish pins it.

aBrokerWithARecoveryToRunRefusesTheChangesOnly pins #1220: while a recovery is required, a monitoring request is answered at once, and a local change does not reach the replication server until the recovery no longer is. It is red on f06769e, where the request waits its 10 seconds and gets nothing.

issue (non-blocking): NO_SUCH_OBJECT from a backend that a failed import left deregistered. Confirmed. Your isBaseEntryGone() is already in the domain, as baseEntryIsAbsentFromALiveBackend(), which findEntryDN() reads a NO_SUCH_OBJECT with; the round reads it the same way now (:7465-7478). The lock refusal itself is a short window: importConfig is set before preBackendImport(), so the finally runs closeBackendImport(), which enables the backend back. The longer one is a total update into this replica which goes through: the backend is away for the whole import, the domain stays enabled, and a recovery which runs meanwhile searched each window into NO_SUCH_OBJECT. A recovery publishes nothing during a total update now (see the last point below), and a round whose search finds no backend fails. aSearchWhichFindsNoBackendFailsItsRound: a search which answers NO_SUCH_OBJECT while the base entry is there fails its round and publishes nothing, and the next recovery publishes the change. It is red on f06769e.

suggestion (non-blocking): no test pins the wait before a failed recovery starts the session again, or the reset of its counter. Added aRecoveryWhichKeepsFailingWaitsLongerEachTimeAndForgetsItOnceItSucceeds:

• the search fails twice in a row, each time on a session of its own, and then goes through;
• the log has 1000 ms for the first failure and 2000 ms for the second;
• the time from the second failure to the next search is at least 2000 ms;
• the count is 0 once the recovery is over.

It is red on the mutant without the wait, and on the one without the reset. A session which hands its start over during the wait ends it now: aSessionWhichStartsWhileAFailedRecoveryWaitsIsRecoveredAtOnce - its replication server gets the change it missed within 5 seconds, where the wait of a tenth failure left it to wait 10.

The rest of this round.

• Replication: keep the messages of the local changes done during a recovery, rather than drop them and rebuild them from the historical information #1203: the local changes done during a recovery are kept. While a recovery runs, the pending changes keep the messages of the local changes they push, in order, rather than drop them. The search covers only the changes pushed before the first one kept, up to a CSN no commit moves, and the kept messages go out next. The race between the search and the commits is gone from the common case. A change whose values a later change replaced is published, where the historical information keeps the later one only. The round which ends the recovery holds the local changes back only to publish the ones pushed during the round before, and searches nothing. Past 10000 changes or 32 MiB the recovery keeps none and searches for them as before; that fallback is what the earlier load tests now run, with a recovery which keeps nothing. A round which holds the local changes back and still does not reach the last local change with what was kept - a change which should have been kept is missing - falls back the same way, rather than go round for ever under the lock every local write waits for (aRecoveryWhoseKeptChangesMissOneSearchesForIt). A kept change goes out as not assured, as the changes a recovery rebuilds do: the operation has given up waiting for its acknowledgement by then (LDAPUpdateMsg.setNotAssured(), which drops the cached encoding).
• Replication: a session which starts while the missing change publisher of the previous one still runs gets no recovery of its own #1199: a session which starts while the publisher runs. The session hands the publisher its start and its session, and publishRecovery() takes the session the recovery is for, and refuses the message for any other: the run for the session before can no longer get a later change to the new replication server ahead of the older ones it misses. The start, the end, the hand-over and the exit of the publisher are decided under one lock, taken inside the connect phase lock and inside the lock of the pending changes, and under which neither is taken. This is the hand-over your round 2 named, closed rather than left to Replication: a session which starts while the missing change publisher of the previous one still runs gets no recovery of its own #1199. A publisher which throws an error - which no catch of an exception takes - has the session started again too (aRecoveryWhichThrewAnErrorHasTheSessionStartedAgain).
• Replication: a full map of the operations kept aside for a recovery drops its oldest one, which can be a change the replication server still lacks #1200: the deletes kept aside. What a search finds is the round's own, and the list holds deletes only. The deletes a round publishes stay in it, for the replication server of a later session; your round 2 trim and clear stay out. A delete no replication server took which the cap drops logs ERR_UNPUBLISHED_DELETE_DROPPED (ordinal 334).
• Replication: the deletes kept aside for a recovery outlive a restore or an import of the data, and a later recovery deletes on the other replicas entries this one keeps #1221: the deletes kept aside, and the kept local changes, go with the data. Once the ServerState is read from the data again - as the domain is enabled, or after a total update - the ones the data does not reflect are dropped: it was restored or imported since. What comes after the newest change of this replica that state holds is not in the data, except for a delete, which leaves no historical information for the state to cover, and which the state misses as well when it could not be saved - a delete of the base entry takes the saved state with it. A delete is dropped only if its entry is in the data (theDeletesOfASuffixOutliveAStateWhichCouldNotBeSaved, and the variant for the deletes a recovery kept).
• Replication: a domain which reads back a ServerState with no change of its own moves its CSN generator back, and numbers its next changes as, or before, ones already numbered #1223: the CSN generator does not go back. loadDataState() adjusts the generator with the newest CSN of this replica in the state it reads, and CSNGenerator.adjust(null) - a state with none, as after the delete of the base entry - set it to the clock and its sequence number to 0. The session started next takes it forward to what its replication server holds only, so the next local change was numbered as a change that replication server missed, or before it - a change the recovery was to publish, and one of the two was lost - and before a change of another replica it had followed, by as much as that replica's clock runs ahead. Two tests of this round got the CSN of a delete reissued to the next change, and waited for the clock as a way round it; an adjust with no CSN leaves the generator where it is now, and the waits are gone. aReplicaWhichReadsBackAStateWithNoChangeOfItsOwnNumbersItsNextChangeAfterItsLastOne pins it: with the generator of master, the add of the base entry is numbered as the first delete.
• A total update into this replica. A recovery publishes nothing while one replaces the data: a round refuses, and so does each publish of a round in flight, as what it would publish was done on the data the import replaces. The session the import starts again recovers from the state the new data holds (aRecoveryPublishesNothingWhileATotalUpdateReplacesTheData).

Tests: HistoricalCsnOrderingTest 51/51 green, CSNGeneratorTest 6/6. On f06769e, with the test hooks only, 17 of the 25 new tests of #1195-#1221 are red, each on the part of the change it covers (two more need a method that head does not have). All 33 mutants - each undoes one part of this round - turn their tests red. The three tests of #1223 are red with the generator of master. org/opends/server/replication/**: 3885 tests, all green.

[maximthomas]

praise: The round-3 fixes are where the bugs were, and the suite is green.

• synchronize() takes a local delete that did not go through out of replayOperations before pendingChanges.remove() / pushCommittedChanges() let the later changes out (LDAPReplicationDomain.java:2700-2718).
• ReplicationBroker.publish() now refuses only an UpdateMsg while a recovery is required (ReplicationBroker.java:2372), and a recovery round's NO_SUCH_OBJECT counts as gone through only with baseEntryIsAbsentFromALiveBackend() (LDAPReplicationDomain.java:7475).
• HistoricalCsnOrderingTest (51 cases) and CSNGeneratorTest (6 cases) pass at f7fcf27.

---

issue (non-blocking): A modify or add that a recovery kept is dropped at enable() when the state save in disable() did not write, even though the entry holds the change.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:5810-5817, :5604-5605, opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PersistentServerState.java:140

forgetTheLocalChangesTheDataLacks() drops every kept non-delete change newer than the max CSN of the state it reads back. disable() wrote that state with state.save(). That call returns without writing when the checkpointer's write holds saveLock (disable() does not stop ServerStateFlush), or when updateStateEntry() fails. clearInMemory() then throws away whatever was not written. So a kept modify C that is missing from the saved state gets dropped, although its entry holds it. Now let a local change D come in while the recovery that spans the reload is still running: the round publishes only D, recoveryUntil(D) ends the recovery, and C never reaches a replication server. BASE did publish C, through its windowed search up to D. The give-up in disable() is older than this PR (it dates from #948). The new drop at reload is what turns it into a lost change. "A domain enabled back on its own data loses none" (:5782-5783, and the commit message) is only true when that save wrote. Not run: no test makes the save in disable() lose that race. Also not traced: whether a kept change can be committed after the in-flight write took its snapshot.

Or: at least narrow the claim to a disable whose save wrote. Closing the gap needs one of two changes. Either save() reports whether it wrote and disable() clears the state only once it has, or the drop is based on the data rather than on the max CSN of the saved state.

---

suggestion (non-blocking): No test hands a session start over while a held round publishes the kept changes, so the !startHandedOver() conjunct before keepNoMoreChanges() is not pinned.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:7566, :7579, :7281

The TestBroker cases go through the @VisibleForTesting overload (:7281) with publisher == null, so for them startHandedOver() is always false. The cases with a real publisher hand the start over at other points: in round 2's search (HistoricalCsnOrderingTest.java:1455), after the run (:1528), in the backoff wait (:2178), or with setMaxKeptChanges(0) (:1594). Without the conjunct, a start that arrives during a held round makes the recovery keep no more changes for the rest of its life. The new session's run then searches the historical information, which does not rebuild a kept change whose values a later change replaced. Every case stays green under this mutant (traced by reading, not run):

      if (!endedRecovery && heldBack)

Pin: take aChangeWhoseValuesALaterOneReplacedIsPublished's setup and run it through a real publisher (setMissingChangesPublisherRunHook, kept changes on). From session 1's publishRecovery, while the held round publishes the kept changes, start a session to a second replication server that holds less. Assert that the second replication server gets the replaced change.

---

issue (non-blocking): The comment on Recovery.keep() and the LDAPUpdateMsg.setNotAssured() Javadoc say a kept change is published once its operation has given up waiting for the acknowledgement. Nothing enforces that order.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PendingChanges.java:516-525, opendj-server-legacy/src/main/java/org/opends/server/replication/protocol/LDAPUpdateMsg.java:209, LDAPReplicationDomain.java:2659-2679

keep() runs inside commitAndPushCommittedChanges() (LDAPReplicationDomain.java:2663), before the operation enters waitForAckIfAssuredEnabled() (:2675). The rounds publish kept changes without looking at waitingAckMsgs or the assured timeout. With assured replication on, a kept change can therefore go out, not assured, while its operation is still waiting. No acknowledgement comes back, so the operation waits out the whole assured timeout and logs NOTE_DS_ACK_TIMEOUT. That outcome is the same as BASE, which published nothing during a recovery. What is wrong is the stated order, which appears in the comment, the Javadoc and the commit message.

      /*
       * Published as a recovery publishes what it rebuilds: not assured. Its operation may still be
       * waiting for the acknowledgement when a round publishes it; it then gets none and times out,
       * as it waits as the domain is configured to, on the CSN of the message, and not as the message says.
       */

Or: keep the message assured while its CSN is still in waitingAckMsgs. The operation could then return on the acknowledgement.

---

question (non-blocking): (round 2) Does #1202's fix — bounding the held publish, or letting stop() in — also cover replays that wait on the entry locks of local operations parked in a held round?

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/PendingChanges.java:588, LDAPReplicationDomain.java:7248

A local operation parked in holdBackLocalChangesWhile() keeps its entry lock. A replay of the same entry gets BUSY after LockManager's 9 s, and after 10 attempts it gives up. This is a Minor. It becomes a Major if a held round can outlast ~90 s without a withheld window or a reconnect. I have not re-derived it at this head, where the hold covers only the publish of the kept changes.

---

nitpick (non-blocking): ERR_MISSING_CHANGES_NOT_PUBLISHED_333 gives the backoff as the delay before the next attempt, but a session start that is handed over ends the wait early.

opendj-server-legacy/src/messages/org/opends/messages/replication.properties:722-724, logged at LDAPReplicationDomain.java:949

waitForAStartOrTheBackoff returns as soon as a session hands its start over. It returns at once if the hand-over came during the failed run. The number is an upper bound.

ERR_MISSING_CHANGES_NOT_PUBLISHED_333=Domain "%s" could not publish the changes its replication \
 server misses: %s. It tries again within %d ms - on the session which starts meanwhile, or on one it \
 starts then - and the changes done meanwhile are left to that recovery as well

[vharseko]

Round 5: 0a1cdf7. The branch is rebased onto master (616d5f6, whose change since round 4 touches .github only), and the three earlier commits are unchanged; the commit of round 4 has its message corrected for the two statements below, and its code unchanged.

issue (non-blocking): a kept change dropped at enable() when the save in disable() did not write. Not as described: the state read back is brought up to the data before anything is dropped. loadState() ends with checkAndUpdateServerState() (PersistentServerState.java:413-468), which searches the entries whose ds-sync-hist is past the newest change of this replica in the saved state, and moves the state to the newest change of this replica they hold. It is there for the state a crash leaves behind, and a save which gave its turn up, or failed, is the same case. A change is kept only once it is committed, so its entry holds it, the state read back covers it, and forgetTheLocalChangesTheDataLacks() keeps it; the case you did not trace - a change committed after the snapshot of the write in flight - goes the same way. theLocalChangesARecoveryKeptOutliveADisableWhoseSaveDidNotWrite pins it: after disable() the base entry gets back the state saved before the two kept changes, of the same values, and both reach the replication server. It is red with the update() of that search taken out.

The gap is narrower, and real: checkAndUpdateServerState() returns at once when the saved state holds no change of this replica (:420). Such a state comes back once the base entry, which holds the saved state, was deleted and added back, and no save wrote into it since. Every kept change but the deletes was then dropped, while the deletes stayed - the entry of the base entry deleted is not in the data, the one added back has another entryUUID - so the recovery published the deletes of the suffix without the add of the base entry, and the other replicas lost the suffix. loadDataState() now brings such a state up to the changes of this replica the historical information holds from the first change the recovery kept (bringAStateWithNoChangeOfThisReplicaUpToTheData(), LDAPReplicationDomain.java:5845); a search which does not go through logs ERR_CANNOT_RECOVER_CHANGES, as the one of PersistentServerState does, and the kept changes go then, as they do when that one fails. aSuffixAddedBackDuringARecoveryOutlivesAStateWhichCouldNotBeSavedInIt: the delete of an entry, the delete of the base entry and its add, all kept; disable(); the base entry holds no state; enable(). Without the bring-up the replication server gets the two deletes only. The comment of forgetTheLocalChangesTheDataLacks(), the commit message of round 4 and the description now say what the claim rests on, and that a failed search is the case left.

suggestion (non-blocking): the !startHandedOver() conjunct is not pinned. Pinned by aSessionWhichStartsWhileTheLocalChangesAreHeldBackGetsTheChangesTheRecoveryKept, red without the conjunct. The start is not handed over from publishRecovery(): a session started through a configuration change does not get in while a round holds the local changes back - restartService() stops the broker, and stop() publishes the announcement that the replica goes offline under the lock the round holds. A session which comes up as the broker reconnects does: ReplicationBroker.reStart(), after its replication server went away or for a better one, takes no lock of the pending changes. So the test has a hook run once a round has published the kept changes (setKeptChangesPublishedHook()): in the round which does not hold the local changes back it does two changes of the same values, and in the one which does, it removes the first replication server and waits for the session to the second one to hand its start over. The second replication server is in another group, so the domain, which knows of both, goes to the first one for as long as it is there; it gets every change it misses, the replaced one included. It is created first: two replication servers in one JVM share the changelog backend the first one registers, which goes away with it.

issue (non-blocking): the stated order of a kept change and the end of the wait for its acknowledgement. Confirmed: waitForAckIfAssuredEnabled() waits on needsAck() and waitingAckMsgs, not on the flag of the message, and keep() runs before it. The comment (your wording), the Javadoc of setNotAssured(), the Javadoc of aKeptChangeIsPublishedNotAssured, the description and the commit message of round 4 are corrected; my round-4 reply said the same, and was wrong. The message is not kept assured while its CSN waits: an operation which waits longer than the recovery takes ends as on master, and the gain - the operations whose change a round reaches within the assured timeout - would need a check against waitingAckMsgs on each publish, and a test with the acknowledgements of a real replication server.

question (non-blocking): #1202 and the replays waiting on the entry locks of local operations parked in a held round. As in round 2: no, #1202 and #1210 bound stop() only, and the replay side is in #1209. Re-derived at this head: a held round publishes from memory the changes kept during the round before it, which itself published from memory - at most 10000 changes or 32 MiB for the whole recovery - and runs no search (noSearchRunsWhileTheLocalChangesAreHeldBack). A held round searches only once the recovery keeps nothing - past the cap, or after a round whose kept changes miss one - and then covers the changes done during the round before it, which searched one window. To last ~90 s it would have to take that long to search and publish ten seconds of changes, which a recovery that slow never catches up to; or the window is withheld, or the session reconnects, which you put aside. It stays a Minor.

nitpick (non-blocking): ERR_MISSING_CHANGES_NOT_PUBLISHED_333 gives an upper bound as the delay. Taken as you wrote it ("within"); aRecoveryWhichKeepsFailingWaitsLongerEachTimeAndForgetsItOnceItSucceeds reads the new text.

Tests: HistoricalCsnOrderingTest 54/54 green, CSNGeneratorTest 6/6. The three mutants of this round turn their tests red: without the conjunct (the held round falls back to the search, and the second replication server misses the replaced change), without the bring-up of a state with no change of this replica (f7fcf27 for that test: the two deletes only), and without the update() of checkAndUpdateServerState() (nothing reaches the replication server). org/opends/server/replication/**: not run again locally for this round: the run was cut short as the machine ran out of memory, after other test JVMs on it had taken the ports of its servers; the CI runs it.

[maximthomas]

praise: The new bring-up sits exactly where #1221's add of the base entry was lost, and both round-5 mutants the description names turn their tests red.

• Deleting bringAStateWithNoChangeOfThisReplicaUpToTheData(); at LDAPReplicationDomain.java:5773 turns aSuffixAddedBackDuringARecoveryOutlivesAStateWhichCouldNotBeSavedInIt red (the add of the base entry is missing from the changelog). Dropping !startHandedOver() at :7659 turns aSessionWhichStartsWhileTheLocalChangesAreHeldBackGetsTheChangesTheRecoveryKept red. Both measured at this head, HistoricalCsnOrderingTest 54 tests.
• In docker-convergence (rs534) at this head, both replication servers' changelogs hold every change of every replica (changelog/cv-r0.txt, cv-r1.txt in the artifact: 0 missing), cv-d3 included after 29 reconnections.