mirror of https://github.com/OpenIdentityPlatform/OpenDJ.git

Valery Kharseko
yesterday 3619801f5cfce1e8852b4553b0c4eb4ad19853a8
refs
author Valery Kharseko <vharseko@3a-systems.ru>
Wednesday, October 21, 2026 08:41 +0200
committer GitHub <noreply@github.com>
Wednesday, October 21, 2026 08:41 +0200
commit3619801f5cfce1e8852b4553b0c4eb4ad19853a8
tree d0320473a453864ad104c9e6f4a75646528e43fe tree | zip | gz
parent c1019f86dc29b12205408e01733b433c8a33de0b view | diff
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.
1 files modified
25 ■■■■■ changed files
opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/HistoricalCsnOrderingTest.java 25 ●●●●● diff | view | raw | blame | history