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

Valery Kharseko
16 hours ago d954c4c53f3d626f2a60c86eed653569dfd907fe
refs
author Valery Kharseko <vharseko@3a-systems.ru>
Monday, October 19, 2026 16:48 +0200
committer GitHub <noreply@github.com>
Monday, October 19, 2026 16:48 +0200
commitd954c4c53f3d626f2a60c86eed653569dfd907fe
tree ced0bd164e2bc593b96488560ebc0cb093227b22 tree | zip | gz
parent b8a4d35f03b7fa5cf0d80a9424224f6ddeb3b3f9 view | diff
[#1215] Check a replication server the topology already has against the configured ones again, from the URL it is known with or the one it now comes with (#1218)

Fixes #1215

## Problem

A directory server left the replication server it was connected to every
two minutes, "because it is more suitable" to go to another one (msgID
188, `RS(X) was not configured locally on DS(N), but at least one other
RS was`), and connected straight back to it (msgID 62), for as long as
it ran. Each of those reconnections restarts the session, and under load
a reconnection is where #1206 lost changes.

`Topology.computeRSInfos()` decided whether a replication server is
locally configured only when the server first appeared in the topology
of the broker. A server the previous topology already had only went
through `ReplicationServerInfo.update()`, which keeps the flag and the
URL the server was first known with.

- A server a `TopologyMsg` named before the replication-server list of
the domain had it stayed not locally configured once the list got it.
That is the order of the background join of the Docker image, and of a
replication server added to a running topology.
- The restart of the session that the new list asks for keeps the
topology: `connectAsDataServer()` merges the infos of the phase-one
handshakes into it, and the periodic check of `receive()` then rejects
the server the broker is connected to by its first criterion,
`filterServersLocallyConfigured()`.
- The next connection evaluates the fresh phase-one infos, which take
every configured server as locally configured, and picks the same server
again.

The same loop follows, on master as well, when a configured server
announces a URL which is not the configured one (a port mapped by NAT or
by Docker) and the broker first hears of it from a `TopologyMsg`,
because it was down when the broker connected: the phase-one handshake
reaches it at the configured URL, but the topology keeps the announced
one.

## Change

`computeRSInfos()` now sets the flag of a server it updates again, in
the new `updateLocallyConfiguredFlag()`:

- the server is locally configured when the URL it is known with, or the
URL it comes with in the new topology, is one of the configured ones; in
the second case it is known with that URL from then on;
- a server which has the flag, but whose URL the configured ones no
longer have, is checked once more, resolving host names, with the URL it
is known with and then with the one it comes with, which is the URL a
new server is checked with; found so, it is known with the configured
URL from then on; any other loses the flag, and keeps the URL it is
known with;
- a new server is checked as before, by `setLocallyConfiguredFlag()`.

The lookup of a configured URL equivalent to a given one is now
`findEquivalentConfiguredUrl()`, which both `setLocallyConfiguredFlag()`
and the third check use.

The first two checks of a known server compare the strings only, and
resolve no host name. A server found locally configured is known with
the configured URL, and a phase-one handshake names a server with the
configured URL it reached it at. Resolving the hosts of every server on
every `TopologyMsg` would put a lookup per server and per configured URL
on the thread which receives the updates, and
`HostPort.isEquivalentTo()` returns false on an `UnknownHostException`:
a resolution failing for a moment would take the flag from the server
the broker is connected to, which is the reconnection this change is
about. A server with no URL (protocol version < 4) is checked apart: the
configured URLs are a sorted set, which throws on `contains(null)`.

The third check is there for an entry of the configuration written
another way, with an equivalent URL. The restart of the session which
that change asks for gives the flag back through the phase-one
handshake, but the restart is refused while a total update into this
replica runs, and in the request phase of the total update the periodic
check of `receive()` still runs: without the third check it would leave
the server the broker is connected to. The URL the server comes with is
looked up only when the one it is known with is not found: a server
known with the configured URL a phase-one handshake reached it at may
come with another one (a port mapped by NAT or by Docker), and the entry
may be written again as an equivalent of that one. Only a server with
the flag gets to the third check, and a server found so is then known
with the configured URL, so it costs at most two checks per change of
the configuration and per server which lost its URL. A resolution
failing at that moment takes the flag.

A change of the flag or of the URL of a known server is traced. The
previous topology holds the same `ReplicationServerInfo` object, so the
comparison of the two topologies in `topologyChange()` cannot show it.

Left as is: a server without the flag, known with or coming with a URL
which is only equivalent to a configured one, after resolution, gets the
flag at the next connection. A change of the list restarts the session,
and the phase-one handshake then gives the configured URL.

A server removed from the list was already left out: the restart builds
the topology from the phase-one infos, which only have the configured
servers, and `computeRSInfos()` keeps only the servers it is given. The
test of that case pins the same semantics in `Topology` itself.

## Tests

`ReplicationBrokerTest`, 13 new tests, 24 runs: the tests with a data
provider run through each of the two constructors of `Topology`.

| # | Test | What it pins |
|---|---|---|
| 1 |
`topologyShouldStampRSAddedToTheConfigurationAfterThePreviousTopology` |
A server a `TopologyMsg` named while the list lacked it is locally
configured, with the configured URL, once the list has it and the new
topology names it with that URL; the server the broker is connected to
still is. Red on master. |
| 2 |
`topologyShouldNotStampRSRemovedFromTheConfigurationAfterThePreviousTopology`
| A server the list no longer has is not locally configured. Red on
master. |
| 3 | `topologyShouldStampRSReachedAtAConfiguredURLItDoesNotAnnounce` |
`TopologyMsg` with the announced URL, then a phase-one handshake at the
configured URL, then a `TopologyMsg` with the announced URL again:
locally configured from the handshake on, with the configured URL. Red
on master. |
| 4 | `topologyShouldKeepStampedAConfiguredRSWithoutResolvingItsHost` |
A known server with a configured URL whose host cannot be resolved
(`rs.invalid`) stays locally configured, with that URL, when the new
topology names it with another URL. Skipped where the resolver answers
for `rs.invalid`. |
| 5 |
`topologyShouldStampRSAddedToTheConfigurationWithTheURLItAnnounces` |
The same as 1 with the URL the server was first known with, which is the
input of the background join of the Docker image. Red on master. |
| 6 | `bestServerCheckShouldNotRejectTheRSAddedToTheConfiguration` |
`computeBestReplicationServer()` does not reject the server of 5 as not
locally configured once the broker has reconnected to it. The other
server is not on this host, so the same-host filter keeps only this one:
the case pins the filter of the locally configured servers, not which
server the broker is connected to. Red on master. |
| 7 | `topologyShouldKeepStampedRSWhoseConfiguredURLIsWrittenAnotherWay`
| A locally configured server whose entry is written `127.0.0.1:8989`
instead of `localhost:8989` keeps the flag, with the new URL. Red on
master. |
| 8 | `topologyShouldNotResolveTheURLAKnownRSComesWith` | A known server
without the flag which comes with a URL only equivalent to a configured
one stays without it. |
| 9 | `topologyShouldNotResolveTheURLAKnownRSIsKnownWith` | A known
server without the flag whose URL gets an equivalent in the list stays
without it. |
| 10 |
`topologyShouldStampRSAddedToTheConfigurationWithTheURLItIsKnownWith` |
A known server which comes with another URL, while the list now has the
URL it is known with, is locally configured, with that URL. IP literals
with distinct ports, so that no resolver can make it pass without the
first check. Red on master. |
| 11 | `topologyShouldNotStampKnownRSWithoutURL` | A known server with
no URL is not locally configured, and the check does not ask a sorted
set of configured URLs, as the configuration is, for a null one. |
| 12 |
`topologyShouldKeepStampedRSWhoseConfiguredURLIsWrittenAsTheURLItComesWith`
| A locally configured server, known as `localhost:28989` from the
phase-one handshake, comes with `localhost:8989`, and the entry is
written `127.0.0.1:8989`: it keeps the flag, with the new URL. Red on
master. |
| 13 | `topologyShouldNotStampRSWhoseConfiguredURLNamesNeitherOfItsURLs`
| The same server, with the entry replaced with `127.0.0.1:18989`,
equivalent to neither of its URLs: it loses the flag, and keeps the URL
it is known with. Red on master. |

Mutants of `ReplicationBroker`, each run against the class:

| Mutant | Caught by |
|---|---|
| a known server checked again as a new one, resolving host names | 9,
12 (both constructors), 4 (both constructors, where `rs.invalid` does
not resolve), 3 |
| the flag of a known server can only be raised | 2, 7, 12, 13 (both
constructors) |
| the flag set without the URL it was found with | 1 (both
constructors), 3 |
| only the URL the server is known with counts | 1 (both constructors),
3 |
| no first check: only the URL the server comes with counts | 10 (both
constructors), 4 (both constructors, where `rs.invalid` does not
resolve) |
| the URL the server is known with keeps the flag it had | 5, 10 (both
constructors), 6 |
| a server with the flag which lost its URL is not checked again | 7, 12
(both constructors) |
| a server without the flag which lost its URL is checked again as well
| 8, 9 (both constructors) |
| the URL the server comes with matched by resolving host names | 7, 8,
9, 12 (both constructors) |
| the third check looks up only the URL the server is known with | 12
(both constructors) |
| a server which loses the flag takes the URL it comes with | 13 (both
constructors) |
| a server with no URL looked up in the configured URLs | 11 (both
constructors) |

Not pinned: the trace. Dropping it leaves the class green; it is a
diagnostic, and its format and its six arguments were checked by
reading.

## Verification

- `ReplicationBrokerTest`, run directly with TestNG, JDK 21: 37 tests, 0
failures; 16 of them fail on master. On a resolver which answers for
`rs.invalid` (`-Djdk.net.hosts.file`), test 4 is skipped in both runs
and the other 35 pass.
- `org/opends/server/replication/**` in the reactor (`-pl
opendj-server-legacy -am`), on master b8a4d35f03, JDK 27, macOS: 3946
tests, 0 failures, 0 errors, 0 skipped.
2 files modified
481 ■■■■■ changed files
opendj-server-legacy/src/main/java/org/opends/server/replication/service/ReplicationBroker.java 131 ●●●●● diff | view | raw | blame | history
opendj-server-legacy/src/test/java/org/opends/server/replication/service/ReplicationBrokerTest.java 350 ●●●●● diff | view | raw | blame | history