[vharseko]

Fixes #1206
Fixes #1224
Fixes #1225

Problem

A replication server stopped sending a directory server the changes of another replica, and went on minutes further. The changes in between were in the changelog of every replication server, never reached that directory server, and missing-changes still reached 0.

A handler which is behind reads the changes for its consumer from the changelog. MessageHandler.getNextMessage() takes it back to its in-memory queue once the queue holds the first change of what it read, which is the oldest change the consumer lacks. Holding that change does not mean holding the ones after it:

• The queue is ordered by CSN, and drops its oldest changes when it is full.
• A change which reaches the queue late with an older CSN goes in front: one relayed by another replication server, or published again by a recovery.
• Behind it, the queue can lack newer changes of another replica: the ones it dropped before that change came, or the ones which reached the domain before the handler was registered.

That change is then the oldest one the consumer lacks, and it is in the queue. The handler went back to the queue, the consumer was given the changes the queue still held of the other replica, and its state moved past the ones in between. The changelog is read again only for a change the state of the consumer does not cover (#963), so they were never sent.

A full queue keeps such a change because it is bounded in bytes as well and drops whole changes: the room a large change leaves takes a small one in.

The changelog is read without the lock of the queue, so a change arriving in between is lost the same way:

• with an older CSN than the first change read, it was dropped along with the changes before that one, which the consumer has;
• into a full queue as its oldest change, it was dropped at once, and a handler whose read gave nothing went back to the queue all the same.

This fits what #1206 found:

• The replica skipped was connected to the same replication server as the directory server. The replicas whose changes came through the other replication server were not skipped.
• Both jumps of the description landed on the last 10 to 13 s of the skipped replica, which is what the queue held of it.
• The new handlers of cases 2 and 3 of the comment skipped from the moment they connected: their queues had nothing from before.

#1225 is the same with a hole of one change, in the combined topology of the convergence check. The replication server of cv-c2 never sent its directory server one delete of cv-c1, though it sent the changes of cv-c1 on both sides of it and every add and delete of the other replicas. That is test 1 below: the queue dropped the change, and a change of another replica with an older CSN came in after it.

Change

The handler records, for each replica, the newest change of it the queue may lack:

• what the domain held when it handed the queue its first change: the domain hands a registered handler every change it receives which is meant for its consumer, and writes a change to the changelog before it hands it over;
• every change the queue drops when it is full.

It goes back to the queue only once the state of the consumer covers them, and once the queue holds nothing before the first change read that the consumer lacks (MsgQueue.holdsAChangeBefore()). It reads the changelog until then. All three ways back to the queue check what the queue lacks: the one which finds the first change read in the queue, the one taken when the read gave nothing, and the one taken when both queues are empty and nothing is found after the state of the consumer.

• Only the changes a read of the changelog can still give count, for what the queue lacks and for what it holds before the first change read. A change the changelog no longer holds, as one of a changelog cleared for a new generation id, is left out once the consumer has every change of its replica the changelog holds. Otherwise a read which gives nothing but replica offline messages would be made again and again, with no wait.
• A replica offline message in the queue is left out as well: no state of a consumer ever holds its CSN.
• When the queue holds every change after the state of the consumer, nothing changes: what it lacks is older than the oldest change the consumer lacks.
• For the handler of a peer replication server, what the domain held includes changes of replicas connected to other replication servers, which the domain never hands it. The handler reads those the state of the peer lacks from the changelog once, up to what the domain held then, before it goes back to its queue. The peer gets them from the replication servers they come from as well. Its changelog keeps one copy of each, and with Changelog: a cookie based persistent search is notified twice of a change its replication server receives twice #1224 fixed below, its cookie based persistent searches are not told of them twice either.
• Changes the domain does not hand a handler on purpose, to a directory server with another generation id or being initialized, are not recorded. Such a directory server ends that status only with a new session (StatusMachine lets neither status go to normal): after its import (ReplicationDomain, broker.reStart(false)), or once the changelog is cleared for its generation id, when the replication server closes its session (DataServerHandler.changeStatusForResetGenId()).

Cookie based persistent searches (#1224)

FileChangelogDB.publishUpdateMsg() notified the cookie based persistent searches of cn=changelog of every change it was given, also of one its replica DB did not take: Log.append() drops a record whose key is not after the newest one, which is what a change the changelog already holds is. A change a replication server receives twice reached those searches twice. LogFile.append(), Log.append() and FileReplicaDB.add() now tell whether the record was appended, and publishUpdateMsg() notifies the persistent searches only of a change the replica DB took.

Not addressed here, from the comment on #1206:

• case 4: the span all three streams skipped after the changes sent again at the reconnection of 23:08:57;
• RemotePendingChanges.putRemoteUpdate() taking a change sent again which the state of the directory server covers.

Tests

MessageHandlerCatchUpTest (new) drives a handler no domain feeds from one thread, as the writer of a session does, over the changelog of a real replication server. A change arrives the way ReplicationServerDomain.put() hands it over: written to the changelog, then given to the queue. For the reads, the domain of the handler can hand it a change right after it has read the changelog, or once a read gave nothing.

#	Test	What it pins
1	aChangeTheFullQueueDroppedIsSentWhenAnOlderOneCameInAfterIt	The queue dropped a change of A, then took in a change of B with an older CSN, in the room the drop left: the consumer is sent the dropped change. Red on master.
2	aChangeWhichReachedTheDomainBeforeTheHandlerIsSentWhenAnOlderOneIsQueued	The queue never had the changes of A which reached the domain before it, then got a change of B with an older CSN and a later change of A: the consumer is sent them. Red on master.
3	aChangeOfAReplicaTheQueueIsNeverHandedIsSentWhenAnOlderOneIsQueued	The same with no later change of A. Red on master.
4	aChangeWhichArrivedWhileTheChangelogWasReadIsSent	A change of B with an older CSN than the first change read arrives once the changelog is read: the consumer is sent it. Red on master.
5	aChangeTheFullQueueDroppedWhileTheChangelogWasReadIsSent	A change the full queue drops at once arrives once a read gave nothing: the consumer is sent it. Red on master.
6	aHandlerWhoseQueueHoldsWhatTheConsumerLacksFollowsTheQueueAgain	The queue of test 1, for a consumer which has the dropped change: the handler goes back to the queue.
7	aDroppedChangeTheChangelogNoLongerHoldsDoesNotHoldTheHandlerBack	The queue dropped a change the changelog no longer holds, of a replica which writes nothing more: the handler goes back to the queue.
8	aReplicaOfflineMessageDoesNotHoldTheHandlerBack	A replica offline message in the queue before the first change read does not keep the handler from it, also once its replica has written a change the consumer lacks.
9	aQueuedChangeTheChangelogNoLongerHoldsDoesNotHoldTheHandlerBack	The queue holds a change the changelog no longer holds, and the changelog gives nothing but a replica offline message: the handler goes back to the queue instead of reading the changelog again and again. Red before round 2.
10	aDroppedReplicaOfflineMessageDoesNotHoldTheHandlerBack	The full queue dropped a replica offline message, for a consumer which has every change of its replica: the handler goes back to the queue.
11	aHandlerWhichCaughtUpFromTheChangelogFollowsTheQueueAgain	A handler which has been handed a change catches up from the changelog until a read gives nothing: the change handed after that read is served from the queue.
12	aHandlerWhichReadTheChangesOfAReplicaItIsNeverHandedFollowsTheQueueAgain	The domain held a change of a replica the queue is never handed when it handed the queue its first change: the handler reads it from the changelog, then goes back to the queue once a read gives nothing. Red on master.

For #1224:

Test	What it pins
ChangelogBackendTestCase.aChangeTheChangelogAlreadyHoldsIsNotNotifiedAgainToACookieBasedPersistentSearch	A directory server publishes a change, the same change again, then a newer one: a changes-only cookie based persistent search of cn=changelog is notified of each once. Red on master: [csn1, csn1, csn2].
LogTest.appendTellsWhetherTheRecordWasAppended	A record whose key is not after the newest one is not appended, whether the head log file is rotated or not, and append() says so.

Mutants of MessageHandler and MsgQueue, each run against MessageHandlerCatchUpTest:

Mutant	Caught by
the changes the queue drops are not recorded	1, 5
what the domain held when it handed the queue its first change is not recorded	2, 3, 12
what the domain holds is recorded at every change handed to the queue	6, 7, 8, 10, 11, 12
a change the queue dropped counts when the changelog no longer holds it	7, 10
a change of the queue before the first change read counts when the changelog no longer holds it	9
a change counts when the consumer has every change of its replica the changelog holds	10
the queue before the first change read is not checked	4
a replica offline message counts as a change the consumer lacks	8
the handler never goes back to the queue on finding there the first change read	6, 7, 8, 9, 10
the handler goes back to the queue when the read gave nothing, whatever the queue lacks	5
a handler which has been handed a change never goes back to the queue when a read gives nothing	11, 12
the third way back to the queue is not checked	none: no state the handler reaches lets the check decide on that way

Mutants of the fix of #1224, each run against its two tests:

Mutant	Caught by
the persistent searches are notified whatever the replica DB took	ChangelogBackendTestCase
the replica DB tells it added a change the log did not append	ChangelogBackendTestCase
the log file tells it appended a record breaking the key ordering	both
the log tells it appended a record when its head log file is not to be rotated	both
the log tells it appended a record when it gave up a rotation	LogTest

Verification

Run directly with TestNG, JDK 21, macOS:

• MessageHandlerCatchUpTest: 12 tests, 0 failures. On master 6 of them fail (1-5, 12); at the head of round 1, ad3e21ad6d, 1 fails (9).
• The two tests of Changelog: a cookie based persistent search is notified twice of a change its replication server receives twice #1224: 0 failures. The one of ChangelogBackendTestCase fails on master.

In the reactor (-Pprecommit verify -pl opendj-server-legacy -am, javadoc included), JDK 27, macOS, org/opends/server/replication/** and ChangelogBackendTestCase: 3879 tests, 0 failures, 0 errors, 0 skipped. Among them is MissedUpdateRecoveryTest (#963), which drives the same handler through real sessions: 8 tests, 0 failures.

[maximthomas]

praise: The gate sits on both roads where #1206 lost changes, and the tests prove what the description says they prove.

• queueLacksUpTo records both sources of loss: the domain state at the first handout (MessageHandler.java:160-166) and every change a full queue drops (queueLacksUpTo.update(msgQueue.removeFirst().getCSN()), :173). It is checked on the empty-read road (:388) and on the first-change-found road (:422-424).
• MessageHandlerCatchUpTest measured on JDK 21.0.2: 8/8 green at the head. With MessageHandler/MsgQueue from master, cases 1-5 fail on their send assertions and 6-8 pass, as described. Description mutants (a) record at every handout, (b) count offline messages and (c) never go back on the first change are each killed by exactly the cases the table names.

---

issue (blocking): MsgQueue.isCoveredBefore() has no exemption for a queued change that the changelog no longer holds, so the writer thread can re-read the changelog with no wait.

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MsgQueue.java:194-207, MessageHandler.java:411-424

Take a late DS handler whose queue still holds a change C1 of replica A that its consumer lacks, at the moment a generation id is reset. clearDbs() empties the changelog but no handler queue (MessageHandler.java:809, in shutdown(), is the only msgQueue.clear()). changeStatusForResetGenId puts the DS in BAD_GEN_ID, so nothing is handed to the queue that could evict C1. Then a replica C that wrote after the clear shuts down cleanly before the handler's first empty read. From then on, every fresh cursor re-serves C's ReplicaOfflineMsg: FileChangelogDB.getOfflineCSN() (:843-854) returns it whenever the start CSN is older, and updateServerState() never records its CSN. The non-contributing road (:413-418) moves it into msgQueue, and isCoveredBefore(offline, serverState) fails on C1. lateQueue is now empty and following false, so while (activeConsumer) opens a new cursor at once. The writer thread spins until C writes again or the DS gets a new session. On master, contains(msg) alone set following, consumeUpTo() dropped C1 and the handler waited on its queue. A peer-RS handler spins the same way between arrivals until a full queue evicts C1. The description says "A change the changelog no longer holds ... is left out", and consumerHasWhatTheQueueLacks() does leave it out (newest != null && ...); isCoveredBefore() does not. I traced this by reading the code; I did not run it. The trigger is a timing window, so it is rare.

// MsgQueue.java
public boolean isCoveredBefore(UpdateMsg msg, ServerState state, ServerState inTheChangelog)
{
  synchronized (lock)
  {
    for (UpdateMsg before : map.headMap(msg.getCSN()).values())
    {
      final CSN csn = before.getCSN();
      final CSN newest = inTheChangelog.getCSN(csn.getServerId());
      final CSN sent = state.getCSN(csn.getServerId());
      if (before.contributesToDomainState() && !state.cover(csn)
          && newest != null && (sent == null || newest.isNewerThan(sent)))
      {
        return false;
      }
    }
    return true;
  }
}

// MessageHandler.java:422-424
if (msgQueue.contains(msg)
    && consumerHasWhatTheQueueLacks()
    && msgQueue.isCoveredBefore(msg, serverState, replicationServerDomain.getLatestServerState()))

A change that arrives late with an older CSN (case 4) is written to the changelog before add(), so newest > sent for its replica and it still holds the handler back. The other gate, consumerHasWhatTheQueueLacks() (:188-200), already has this exemption; one getLatestServerState() snapshot can feed both.

Pin: a MessageHandlerCatchUpTest case that queues a contributing change of a replica the changelog holds nothing of, ahead of a ReplicaOfflineMsg the changelog serves. At the head getNextMessage() never returns; with the fix the handler follows its queue.

---

suggestion (non-blocking): No test pins the (sent == null || newest.isNewerThan(sent)) arm of consumerHasWhatTheQueueLacks().

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MessageHandler.java:195

This arm is what lets a peer-RS handler go back to its queue after its full queue dropped a ReplicaOfflineMsg (ReplicationServerDomain.java:440 queues them for RS handlers). The drop records the offline CSN in queueLacksUpTo, and no consumer state ever covers that CSN. Every lacked CSN the eight cases build is at most the changelog's newest for its replica, so the arm never decides anything. Measured: this mutant passes 8/8 at the head (JDK 21.0.2).

// survives MessageHandlerCatchUpTest 8/8
if (!serverState.cover(lacked) && newest != null && true)

Pin: hand a small queue a ReplicaOfflineMsg of replica B, then enough changes of A to drop it, with a consumer that has every change of B in the changelog. The handler must follow its queue again. It does at the head and stays on the changelog under the mutant.

---

suggestion (non-blocking): Once the queue has been handed a change, nothing pins the empty-read road back to the queue.

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MessageHandler.java:388

None of the eight cases reaches the true arm of isMsgQueueBelowThreshold() && consumerHasWhatTheQueueLacks() with a non-empty queueLacksUpTo. MissedUpdateRecoveryTest.aPeerReplicationServerIsLeftAlone never hands its peer handler a change, so the peer-RS one-time read the description promises ("reads those ... once ... before it goes back to its queue") has no test either. Measured: this mutant passes 8/8 at the head (JDK 21.0.2). Under it, a handler that has been handed a change never follows again from an empty read and re-reads the changelog with no wait.

// survives MessageHandlerCatchUpTest 8/8
if (isMsgQueueBelowThreshold() && !queueHandedAChange)

Pin: a case that hands the queue a change, lets the consumer catch up from the changelog until a read gives nothing, and asserts that the next change handed is served from the queue. Running the same case with a replica the queue is never handed also covers the peer-RS read up to the first-handout snapshot.

---

question (non-blocking): Is a duplicate ECL notification on the peer acceptable for the peer-RS one-time read?

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MessageHandler.java:160-166

The first-handout snapshot includes replicas behind a third RS, and put() never hands those to a peer-RS handler (ReplicationServerDomain.java:362). So the handler sends the peer every such change up to the snapshot from the changelog. The description says the peer "takes no change it holds". Its replica DB does drop the copy silently (Log.java:448-452), but FileChangelogDB.publishUpdateMsg() (:919-928) calls notifyCookieEntryAdded regardless, so each copy reaches the peer's cookie-mode persistent searches a second time. This happens at most once per peer session, and master re-sends a shorter stretch the same way. It stays Minor whatever the answer, but the description should name this consequence.

---

suggestion (non-blocking): A third road back to the queue has no gate, but the description says "Both ways back to the queue check what the queue lacks".

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MessageHandler.java:452-458, :653-670

When lateQueue and msgQueue are both empty and findOldestCSNFromReplicaDBs() returns null, following is set with neither check. After a successful read this is equivalent to the gate: null means no replica holds a contributing record after the consumer's state. But the method's catch (ChangelogException e) also returns null (:666-669), and then the handler goes back to its queue unchecked, as on master. Gate it like the other two roads, or say in the comment at :443-447 and in the description why it needs no gate.

// MessageHandler.java:455, inside synchronized (msgQueue)
if (nextChange == null && consumerHasWhatTheQueueLacks())

---

nitpick (non-blocking): The queueLacksUpTo javadoc says the domain hands a registered handler every change it receives, which is false for two kinds of handler.

opendj-server-legacy/src/main/java/org/opends/server/replication/server/MessageHandler.java:84-86

put() hands a ReplicationServerHandler only DS-originated changes (ReplicationServerDomain.java:362), and hands a DS in BAD_GEN_ID or FULL_UPDATE status nothing (isUpdateMsgFiltered, :471-498). The description adds that such a DS ends that status "with a new session ... or once the changelog is cleared for its generation id". On the reset road it returns to NORMAL on its live handler (DataServerHandler.changeStatusForResetGenId, :106-124, called at ReplicationServerDomain.java:2088), with no new session. Name the changes the domain withholds in the javadoc, and drop the reset road from the description's new-session claim.

[vharseko]

Round 2: 7335990422. The branch is rebased onto master 616d5f6c17 first; the round 1 commit is unchanged (git range-diff: =). Two commits on top: f6144ddc26 for this review, 7335990422 for #1224, filed from your question.

issue (blocking), isCoveredBefore() with no exemption: fixed. Reproduced first: aQueuedChangeTheChangelogNoLongerHoldsDoesNotHoldTheHandlerBack queues a change the changelog does not hold, ahead of a replica offline message which the changelog serves on every read. At ad3e21ad6d, getNextMessage() returned nothing within 5 s, and it was the only one of the 12 cases to fail. Now it returns the offline message and the handler follows its queue.

The two checks now share one condition, consumerLacksFromTheChangelog(csn, inTheChangelog), against one getLatestServerState() snapshot. MsgQueue.isCoveredBefore(msg, state) became holdsAChangeBefore(msg, condition), so the condition lives in MessageHandler only.

Case 8 had to change with it. With the exemption, an offline message of a replica the changelog holds nothing of passes whether or not the contributesToDomainState() check is there, so the case no longer saw that check. Now C writes c0 before its offline message and c3, a change the consumer lacks, after it.

suggestion, the (sent == null || newest.isNewerThan(sent)) arm: pinned. aDroppedReplicaOfflineMessageDoesNotHoldTheHandlerBack is your scenario. Your && true mutant fails it.

suggestion, the empty-read way after a first handout: pinned.

• aHandlerWhichCaughtUpFromTheChangelogFollowsTheQueueAgain: the handler catches up from the changelog until a read gives nothing; the change handed after that read must be served from the queue.
• aHandlerWhichReadTheChangesOfAReplicaItIsNeverHandedFollowsTheQueueAgain: the same with a change of B written before the first handout and never handed. Its CSN is after the first change read, so the handler finds that change in its queue and must still read B first. This is the peer-RS read up to the first-handout snapshot.

Your !queueHandedAChange mutant fails both.

question, duplicate ECL notification: yes, and fixed here as #1224. Confirmed as you traced it. publishUpdateMsg() notified whatever Log.append() did, and the persistent phase lets every CSN through (case PERSISTENT:), so a cookie-mode persistent search gets the entry again. That holds for any change a replication server receives twice, not only this read.

• LogFile.append(), Log.append() and FileReplicaDB.add() now return whether the record was appended, and only an appended change is notified.
• ChangelogBackendTestCase.aChangeTheChangelogAlreadyHoldsIsNotNotifiedAgainToACookieBasedPersistentSearch publishes a change, the same change again, then a newer one. Master notifies [csn1, csn1, csn2], the branch [csn1, csn2].
• LogTest.appendTellsWhetherTheRecordWasAppended covers the return value, with and without rotation.

The read itself stays. Leaving the replicas behind a third RS out of the snapshot would need to tell them from a replica which was connected here and moved away: its earlier changes reach a peer only from this RS. The description names the consequence now.

suggestion, the third way back to the queue: checked now, as you proposed. After a successful read the check changes nothing. When findOldestCSNFromReplicaDBs() fails, the handler reads the changelog again instead of going back to its queue, and a failure that persists reaches the writer, which closes the session.

No test pins this check, and its mutant passes all 12 cases. I found no state the handler reaches where the check decides on that way: after the first handout, both queues are empty while the handler is behind only once stopFollowingWhenChangesAreMissing() has taken a DS handler off an empty queue it was following, and every way into following now checks what the queue lacks.

nitpick, the queueLacksUpTo javadoc: fixed. It now names what the domain withholds: a peer replication server is handed only the changes of the directory servers connected to this one, a directory server nothing while in bad generation id or full update status.

The second half I kept, with references added to the description. Both ways out of those statuses take a new session:

• StatusMachine lets neither status go to normal. From bad generation id it allows only not connected, full update or bad generation id (:124-135); from full update only not connected or full update (:113-123).
• On the reset road, changeStatusForResetGenId() never moves a directory server to normal. If the DS's generation id differs from the new one, the DS goes to bad generation id. If a DS in bad generation id now matches it, the RS sends it a StopMsg, sets NOT_CONNECTED_STATUS and leaves the reconnection to the DS (DataServerHandler.java:153-188).

Mutants, in the description: 12 of MessageHandler/MsgQueue against the 12 cases, all caught but the one of the third way; 5 of the fix of #1224, all caught. Master fails 6 of the 12 cases: 1-5 and 12.

Reactor, org/opends/server/replication/** and ChangelogBackendTestCase, javadoc included, JDK 27: 3879 tests, 0 failures, 0 errors, 0 skipped. The description is updated: the shared condition, the three ways back, #1224, the references for the two statuses, the tests, the mutants and the verification.

[maximthomas]

posted by mistake

[maximthomas]

praise: The round-1 loop is closed where it arose, and every mutant row of the description holds when re-run.

• consumerLacksFromTheChangelog (MessageHandler.java:216-221) counts only the changes a read can still give, on all three roads back to the queue (:408-409, :446-448, :481-482). Case 9, aQueuedChangeTheChangelogNoLongerHoldsDoesNotHoldTheHandlerBack, fails against round 1's MessageHandler/MsgQueue with the head's test (JDK 26).
• Re-run at the head, JDK 26: dropping (sent == null || newest.isNewerThan(sent)) fails case 10; !queueHandedAChange on the empty-read road fails cases 11 and 12; dropping before.contributesToDomainState() in MsgQueue.holdsAChangeBefore fails case 8; replicaDB.add(updateMsg) || true fails the new ChangelogBackendTestCase case, with the duplicate notified twice.
• #1224 is fixed at its source: Log.append and FileReplicaDB.add return whether they appended, and FileChangelogDB.publishUpdateMsg (:926) notifies the cookie persistent searches only on true.