external-software/github_OpenIdentityPlatform_OpenDJ.git

Refuse the local changes in HistoricalCsnOrderingTest only once the missing change publisher of the recovery before has left (#1226)

### Problem


`HistoricalCsnOrderingTest#aSessionWhichStartsWhileARecoveryRunsGetsEveryChangeItsReplicationServerMisses`
(added in #1204) failed once on master: run
[39382279910](https://github.com/OpenIdentityPlatform/OpenDJ/actions/runs/39382279910/job/118494172125),
`build-maven (ubuntu-latest, 21)`, 1 failure out of 33782:

```
HistoricalCsnOrderingTest.aSessionWhichStartsWhileARecoveryRunsGetsEveryChangeItsReplicationServerMisses:1494
the second round of the recovery searches expected [true] but found [false]
```

It is a race in the test, not in the recovery:

- `aChangeBothReplicationServersHold()` ends with
`waitUntilTheRecoveryIsOver()`, which returns once the pending changes
are out of recovery.
- The publisher of that recovery then still runs: it takes its next
start, finds none, and as it leaves, `takeStart()` has the broker take
the local changes again (`broker.setRecoveryRequired(false)`,
`LDAPReplicationDomain` :1045).
- `refuseTheLocalChanges()` set the flag with no lock. When the
publisher reset it afterwards, the delete of `dnC` and the modify of
`dnB`, which the test has the replication server miss, reached it.
- The session started again by `disable()`/`enable()` then found a
replication server holding every change of this replica. It started no
recovery, so the hook on the second search never ran, and the test
waited 30 s on `held`.

The test log shows exactly that. After `enable()`, the only search of
the historical information is the one that loads the state
(`ds-sync-hist>=…c135…08`). There is no `publish loop` and no recovery
search.

The production code is unaffected. Every write of the flag (`:983`,
`:1045`, `:7254`, `:7435`) is under `recoveryLock`, and a session which
starts between the end of the recovery and `takeStart()` hands its start
over to the publisher. Only the test wrote the flag outside that
protocol. The same race applies to the other two calls of
`refuseTheLocalChanges()` that follow a recovery (the tests that call it
right after `aChangeBothReplicationServersHold()`).

### Change

`HistoricalCsnOrderingTest.refuseTheLocalChanges()` sets the flag under
`recoveryLock`, once `rsUpdater` is `null`. It retries for up to 30 s,
the same way `waitUntilAStartIsHandedOver()` reads the publisher. Under
that lock, a `null` publisher means that both `takeStart()` and the
`finally` of `RSUpdater.run()` have already written the flag, so nothing
resets it afterwards. For the tests which refuse the local changes on a
fresh domain, no publisher runs and nothing changes.

### Testing

Local reactor runs (`-pl opendj-server-legacy -am -Dit.test=…`, JDK 27,
macOS):

- RED, without the fix: a temporary harness holds the first run of the
publisher in `setMissingChangesPublisherRunHook` until the test has
called `refuseTheLocalChanges()`, then waits for the publisher to leave.
It fails the same way as CI, `the second round of the recovery searches
expected [true] but found [false]`, with the same log: no recovery
search after `enable()`. A plain sleep in the hook does not reproduce
it, because the reset has to land between the refuse and the modify of
`dnB`.
- GREEN, with the fix and the same harness: `HistoricalCsnOrderingTest`
54/54.
- With the fix only (this PR): `HistoricalCsnOrderingTest` 54/54.

The harness is not part of the PR.

			@@ -2810,10 +2810,31 @@
			* server misses some, up to the pending changes leaving them to the recovery: they are recorded in
			* the domain state only, and the replication server misses them. The recovery the next session
			* starts rebuilds them from the historical information.
			* <p>
			* Once the missing change publisher of an earlier session has left, under the lock it leaves under:
			* it has the broker take the local changes again as it leaves, after the end of its recovery - which
			* is all {@link #waitUntilTheRecoveryIsOver(LDAPReplicationDomain)} waits for - and the replication
			* server would get the changes the test has it miss.
			*/
			private static void refuseTheLocalChanges(LDAPReplicationDomain domain) throws Exception
			private static void refuseTheLocalChanges(final LDAPReplicationDomain domain) throws Exception
			{
			brokerOf(domain).setRecoveryRequired(true);
			final Field recoveryLockField = LDAPReplicationDomain.class.getDeclaredField("recoveryLock");
			recoveryLockField.setAccessible(true);
			final Object recoveryLock = recoveryLockField.get(domain);
			final Field publisherField = LDAPReplicationDomain.class.getDeclaredField("rsUpdater");
			publisherField.setAccessible(true);
			new TestTimer.Builder()
			.maxSleep(30, SECONDS)
			.sleepTimes(100, MILLISECONDS)
			.toTimer()
			.repeatUntilSuccess(() ->
			{
			synchronized (recoveryLock)
			{
			assertNull(publisherField.get(domain), "the missing change publisher of an earlier session has left");
			brokerOf(domain).setRecoveryRequired(true);
			}
			});
			}

			private static PendingChanges pendingChangesOf(LDAPReplicationDomain domain) throws Exception