[#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)
Fixes #1207
## Summary
A replication server took every routable message sent to its own server
ID as a message for itself. A directory server whose domain had the ID
of a replication server therefore never got the total update messages
sent to it through that replication server: the replication server
answered them with an `ErrorMsg`, and the online initialize of the
domain never got through. `dsreplication enable` and the installer could
give a new domain such an ID, because they drew the IDs of domains and
of replication servers from two separate sets.
The replication server now routes those messages to the directory server
that has the ID, reports such a directory server in its error log, and
`dsreplication enable` and the installer draw the IDs of new replication
servers and of new domains from one set.
## Cause
- `ReplicationServerDomain.process(RoutableMsg, ServerHandler)` first
compared the destination with the ID of the local replication server,
and only when they differed looked for a directory server with that ID.
- `ReplicationCliMain` (enable) checked the ID of a new domain only
against the domains of the same base DN, and the ID of a new replication
server only against the replication servers.
`Installer.configureReplication` kept the same two sets. The second `if
(cache1 != null)` of enable added the replication server IDs of the
first topology twice and those of the second topology never.
- Nothing reported the clash: `isAlreadyConnectedToDS` looks only at the
connected directory servers.
## Change
`ReplicationServerDomain`:
- `process()`: a directory server with the destination ID, connected to
this replication server or behind a replication server connected to it,
gets the message. A replication server holds no data to export or
import, so the total update messages routed by ID are for the directory
server. A message is for this replication server only when no directory
server has the ID - and in one more case: the warning a peer replication
server sends when the generation IDs differ
(`WARN_BAD_GENERATION_ID_FROM_RS`, sent from `receiveTopoInfoFromRS`).
The directory server would take that warning for the end of the total
update it is in, so it stays with the replication server. It is told
from the other messages by the ID of the message, in either form that ID
takes on the wire: `<resource name>-<ordinal>` since OpenDJ 3.0, and the
numeric ID of the message descriptor from an OpenDJ 2.6 replication
server (14811213: category SYNC, severity SEVERE_WARNING, ordinal 77).
- `stopServer()`: the handler, not its server ID, tells whether a
directory server or a replication server is leaving. Before, a directory
server with the ID of a connected replication server went down the
replication server branch of `stopServer()`: it left the directory
servers, but the peers were never sent the new topology, so they never
heard that the directory server had left.
- `register(DataServerHandler)` / `register(ReplicationServerHandler)`:
a directory server that connects with the ID of this replication server
or of a connected replication server, and a replication server that
connects with the ID of a connected directory server, are reported with
the new `WARN_REPLICA_HAS_THE_SERVER_ID_OF_A_REPLICATION_SERVER_332`,
which asks to give the domain an ID that no replication server of the
topology uses.
`InstallerHelper.getUsedServerIds(Collection<ServerDescriptor>)` (new):
the IDs of the replication servers and of the replicated domains of
every backend base DN, in one set.
- `dsreplication enable` fills it from both topology caches and both
servers, `Installer` from its topology cache or from the suffixes it
found, and both take the ID of a new replication server and of a new
domain outside of it (`InstallerHelper.configureReplication` and the
`ReplicationCliMain` methods now take one set).
- enable no longer limits the topology cache and the descriptors of the
two servers to the base DNs it is run for, the administration data and
the schema: its searches read all the backends and domains anyway, and
the filter only dropped those of the other base DNs from the
descriptors, and so from the set.
- A new domain no longer takes the ID of a domain of another base DN
either, which was allowed before; the IDs run from 1 to 32766.
- `InstallerHelper.addNewReplicationId(Set<Integer>)` replaces
`getReplicationId`: it takes the new ID outside of the set and adds it
to the set, so the replication server and the domains that setup or
enable create one after the other get different IDs. The four callers,
the replication server and the domain of setup and of enable, no longer
add the ID themselves.
## Why not refuse the directory server
#1207 proposed that the replication server refuse such a directory
server at the handshake. A domain that has the ID of a replication
server today replicates its changes; only its online initialize fails.
Refusing it would cut those domains off the replication entirely. And
the replication server it connects to refusing it would not be enough:
the replication server with the ID kept the messages its own directory
servers sent to that ID wherever the directory server with the ID was
connected, so every replication server of the topology would have to
refuse it. Routing the messages to the directory server and reporting it
repairs the existing clashes instead, and the server ID of a domain,
which is read-only, can be changed when convenient.
## Not changed
- A replication server of an earlier version in the same topology still
keeps the messages for a directory server with its own ID; the warning
tells the administrator to give the domain another ID.
- The assured replication keys the expected acknowledgments by server
ID, so it still counts the acknowledgments of a replication server and
of a directory server with the same ID as one. The warning and the
javadoc of `getUsedServerIds` say so.
- `getUsedServerIds` holds only what the descriptors carry: a server
that `dsreplication` or the installer has no descriptor of (not
registered in the administration data, or not reachable) is not in the
set.
- Nor is a domain whose base DN is not the base DN of a backend, such as
a domain below a backend base DN, which
`LDAPReplicationDomain.isConfigurationAcceptable` accepts:
`ServerDescriptor` gives a domain ID only to the replica of the backend
base DN of the domain. Master had the same gap; whether such a domain
can clash in practice was not traced. The comment in
`updateConfiguration` and the javadoc of both `getUsedServerIds` say so.
- The warning is logged each time such a directory server connects.
- The form of the generation ID warning of OpenDJ 2.6 is read from the
sources of tag `2.6.0` (`ErrorMsg`, `MessageDescriptor.getId()`); no 2.6
peer was run, and OpenDJ 2.0-2.4, whose sources are not in this
repository, were not checked.
## Tests
`ReplicationDomainTest`:
| Case | What it pins | master |
|---|---|---|
| `exportAndImportWithAReplicaHavingTheIdOfItsReplicationServer`
(importer 11, exporter 11) | A replica with the ID of its own
replication server imports and exports through it. | red |
|
`exportAndImportAcross2ReplServersWithAReplicaHavingTheIdOfAReplicationServer`
(sender on either replication server) | The same through two replication
servers, the replica with the ID of the first one connected to the
second, and the other way round. | red |
| `replicaWithTheIdOfItsReplicationServerIsReported` | The warning when
the directory server has the ID of its replication server. | red |
| `replicaWithTheIdOfAConnectedReplicationServerIsReported` | The
warning when it has the ID of a peer of its replication server. | red |
| `replicationServerWithTheIdOfAConnectedReplicaIsReported` | The
warning when a replication server connects with the ID of a connected
directory server. | red |
|
`generationIdWarningOfAPeerIsNotForTheReplicaWithTheIdOfTheReplicationServer`
| The generation ID warning of a peer stays with the replication server,
and the directory server with its ID gets no `ErrorMsg`. | green |
| `onlyTheGenerationIdWarningOfAPeerStaysWithTheReplicationServer`
(message ID of the warning since OpenDJ 3.0, of OpenDJ 2.6, of another
message in either form) | A peer sends an `ErrorMsg` to the ID of the
replication server, a directory server with that ID connected: the
warning in either form stays with the replication server, which logs it,
and the directory server gets any other one. The peer sends `ErrorMsg`s
decoded from bytes, as only those carry a message ID of its choice, and
a last one after each: the directory server gets the first one before
the last one, or never. | red (every row: the replication server keeps
every `ErrorMsg` sent to its ID, the last one too) |
| `replicaWithTheIdOfAPeerReplicationServerIsForgottenWhenItLeaves` |
When a directory server with the ID of a peer replication server leaves,
the other replicas stop seeing it. | red |
`InstallerHelperTest.usedServerIdsAreTheIdsOfTheReplicationServersAndOfTheDomainsOfEveryBaseDN`
pins the set: replication servers and replicated domains of every base
DN, not the domains which are not replicated (new method).
`InstallerHelperTest.newReplicationIdIsTakenOutsideOfTheUsedIdsAndAddedToThem`
pins `addNewReplicationId`: with one ID left free, it returns that ID
and adds it to the set (new method).
`ReplicationServerDomainGenerationIdWarningTest` (new) pins the ID that
tells the generation ID warning of a peer: both forms of it, and not the
ID of another message in either form, nor ordinal 77 with another
severity or of another message file.
`ReplicationCliMainTest` (new) pins
`ReplicationCliMain.getUsedServerIds` (now package-private), the helper
enable fills the set with: the replication servers and domains of both
topology caches and of both servers, and only those of the two servers
when there is no topology cache. The topology caches are Mockito stubs.
It calls the helper directly: the arguments that enable passes to it are
not pinned (see below).
The generation ID case, green on master, pins what the change keeps; it
fails if `process()` only looks for a directory server first. Mutants,
each run against the cases above:
| Mutant | Caught by |
|---|---|
| no warning for the ID of the local replication server |
`replicaWithTheIdOfItsReplicationServerIsReported` |
| no warning for the ID of a connected replication server |
`replicaWithTheIdOfAConnectedReplicationServerIsReported` |
| no warning in `register(ReplicationServerHandler)` |
`replicationServerWithTheIdOfAConnectedReplicaIsReported` |
| the warning for every directory server |
`replicaWithTheIdOfAConnectedReplicationServerIsReported` (a replica
with an ID no replication server has is not reported) |
| `process()` checks the local ID first, as on master | the four export
and import cases |
| `process()` looks for a directory server first, no exception for the
warning | `generationIdWarningOfAPeer…`, `onlyTheGenerationIdWarning…`
(both forms of the warning) |
| `stopServer()` classifies by server ID, as on master |
`replicaWithTheIdOfAPeerReplicationServerIsForgottenWhenItLeaves` |
| `getUsedServerIds` without the replication servers / without the
domains / with the domains which are not replicated |
`usedServerIdsAre…` |
| `isGenerationIdWarningOfAPeer` without the check of the message ID |
`onlyTheGenerationIdWarning…` (every row: no other `ErrorMsg` of the
peer reaches the directory server) |
| `isGenerationIdWarningOfAPeer` checks only the form since 3.0, as
before review round 1 | `onlyTheGenerationIdWarning…` (OpenDJ 2.6 form)
|
| `isGenerationIdWarningMsgID` without the OpenDJ 2.6 form / without the
form since 3.0 | `generationIdWarningIsToldByTheIdOfItsMessage`; without
the 2.6 form, also `onlyTheGenerationIdWarning…` |
| `getUsedServerIds` reads the first topology cache twice / the second
one twice / leaves out the two servers |
`usedServerIdsAreTheIdsOfBothTopologiesAndOfBothServers` |
| `updateConfiguration` passes the first topology cache twice, as on
master, or leaves out a server | **not caught**: no test runs `enable` |
| `addNewReplicationId` does not add the ID / ignores the used IDs |
`newReplicationIdIsTakenOutsideOfTheUsedIdsAndAddedToThem` |
No test runs `enable` or `InstallerHelper.configureReplication` against
servers. Not pinned, as all three need `enable` against two servers: the
arguments `updateConfiguration` passes to `getUsedServerIds`
(`ReplicationCliMain.java:4793`), enable limiting the descriptors to its
base DNs again (the filters of `updateConfiguration` and
`createTopologyCacheOrNull`), and enable giving each base DN a set of
its own.
## Verification
- `ReplicationDomainTest` and `InstallerHelperTest`: 35/35.
- `-Pprecommit verify -pl opendj-server-legacy -am` (javadoc included)
with `org/opends/server/replication/**` and `org/opends/quicksetup/**`:
3971 tests, 0 failures.
- Review round 1, on master with #1205 and #1212: `-Pprecommit verify
-pl opendj-server-legacy -am` (javadoc included) with
`ReplicationDomainTest`, `InstallerHelperTest`,
`ReplicationServerDomainGenerationIdWarningTest` and
`ReplicationCliMainTest`: 46/46. The 13 classes of
`org/opends/quicksetup/**`, `SuffixDescriptorTest`,
`ArgumentParserToolsTestCase` and the two new classes, each in a JVM of
its own: 170 tests, one failure, `AdsTrustStoreInstallTestCase`, whose
server could not bind the administration port 65534 that another server
on the machine held; 6/6 when run again. The 7 new mutants are each
caught by their case.
- Review round 2, on master with #1217, #1214 and #1192: `-Pprecommit
verify -pl opendj-server-legacy -am` with the same four classes built
the module and its javadoc, and passed `InstallerHelperTest` (13) and
`ReplicationServerDomainGenerationIdWarningTest` (8);
`ReplicationDomainTest` could not start its server, as another server on
the machine held the administration port 65534, and the run was stopped
there. On the classes of that build, each in a JVM of its own:
`ReplicationDomainTest` 27/27 (at the second attempt, the first one
having lost port 65534 again) and `ReplicationCliMainTest` 2/2. The new
case on master's `ReplicationServerDomain`: 4 of 4 rows red. Four
mutants each fail it: `isGenerationIdWarningOfAPeer` checking only the
form since 3.0 (the OpenDJ 2.6 row), `isGenerationIdWarningOfAPeer`
without the check of the message ID (every row), `process()` without the
exception for the warning (both rows of the warning),
`isGenerationIdWarningMsgID` without the OpenDJ 2.6 form (the OpenDJ 2.6
row).
- The reproduction of #1207 on a package of this branch, before the
changes to `stopServer()`, to the generation ID warning and to the
filter of enable: the domain of B with server ID 4242, the replication
server of A with replication server ID 4242, `dsreplication initialize`
ends in 4 seconds and B holds the 22 entries of A (on master: never
ends).
- The Docker replication test (`docker-test-replication.sh`) was not run
locally.