An empty domain map is left behind by a replica DB creation which bails out

[vharseko]

FileChangelogDB.getOrCreateReplicaDB() inserts a domain map, and announces the domain to every
registered multi domain cursor, before it can know whether it will create anything in it:

// FileChangelogDB.java:229-241
final ConcurrentMap<Integer, FileReplicaDB> newValue = new ConcurrentHashMap<>();
final ConcurrentMap<Integer, FileReplicaDB> previousValue = domainToReplicaDBs.putIfAbsent(baseDN, newValue);
...
// we just created a new domain => update all cursors
for (MultiDomainDBCursor cursor : registeredMultiDomainCursors)
{
  cursor.addDomain(baseDN, null);
}

The caller can then bail out of the creation without ever putting a replica DB in that map:
today when the domain map was concurrently removed (:266), and, once #813 is fixed, when a
shutdown was initiated. The empty map stays in domainToReplicaDBs for the lifetime of the
changelog. On the shutdown path it outlives shutdownDB() altogether, which drained
domainToReplicaDBs before that map was inserted into it.

What that empty domain map is then worth:

• it is a key of domainToReplicaDBs, so getCursorFrom() adds it to every multi domain
  cursor created afterwards (:697-704), i.e. the external changelog walks a domain which has
  no replica DB at all;
• getDomainMap(), getDomainOldestCSNs() and getDomainNewestCSNs() answer for it as they
  would for an unknown domain, so those are unaffected;
• clearDB() iterates the keys and calls removeDomain() on it (:427-430), which reaches
  replicationEnv.clearGenerationId() for a domain the changelog holds nothing for — and, after
  a shutdown, on an environment which is closed.

Proposed fix

Drop the map the caller inserted when the creation it was inserted for does not happen:

// FileChangelogDB.java:266
if (domainToReplicaDBs.get(baseDN) != domainMap)
{
  return null;
}

becomes, together with the shutdown check of #813, a bail-out which first does

domainToReplicaDBs.remove(baseDN, domainMap);

The conditional remove is safe under the monitor of domainMap: a shutdown which had already
seen that map only finds the entry gone when it acquires the monitor, and Iterator.remove()
on an entry which is no longer mapped is a no-op.

Found while analysing #813.

[vharseko]

Three corrections from a deeper pass over the paths involved, before the fix lands:

1. The :266 bail-out leaves no empty map behind today. The identity check domainToReplicaDBs.get(baseDN) != domainMap can only fail because a remover has already unmapped domainMap — so nothing stays behind on that path, and the retry loop of getOrCreateReplicaDB() inserts a fresh map and populates it. The lasting empty map starts to exist only together with the shutdown check of #813, which bails while get(baseDN) == domainMap still holds.

2. The path which does leave an empty map behind today is a FileReplicaDB constructor failure. new FileReplicaDB(...) at :274 throws ChangelogException on an I/O error, and the exception propagates out of getOrCreateReplicaDB() past the freshly inserted, already announced map. The cleanup must cover that exit too, not only the two return null bail-outs.

3. The unconditional remove(baseDN, domainMap) can reintroduce the leak of #813. domainMap is not necessarily the map the caller inserted: the happy path of getExistingOrNewDomainMap() returns a pre-existing map, which may hold the replica DBs of other serverIds. When the shutdown bail-out fires on such a map, removing it races the weakly consistent iterator draining domainToReplicaDBs in shutdownDB(): the safety argument above only covers an iterator which has already returned that map and is blocked on its monitor, not one which has not reached its bin yet — the latter never sees the map, and its replica DBs are never shut down, which is exactly the leak of #813. The remove must therefore be guarded:

if (domainMap.isEmpty())
{
  domainToReplicaDBs.remove(baseDN, domainMap);
}

An empty map is the only one worth dropping, and the only one the drain can safely miss. Wrapping the creation in try/finally with that guard covers all three exits at once — the identity bail-out (where the conditional remove is a no-op), the shutdown bail-out of #813, and the constructor failure — and keeps a map which just received its replica DB, since it is no longer empty by the time the finally runs.

Two smaller notes. Iterator.remove() of a ConcurrentHashMap removes the key unconditionally rather than the entry it returned, so on a concurrently remapped key the drain removes the new map — harmless once the shutdown check of #813 is in, since a post-flip creation bails before putting anything into it, but the "no-op" claim above is exact only for a key which is no longer mapped at all. And dropping the map does not retract the addDomain() already broadcast at :237-240: registered cursors keep the announced domain, which is harmless by the same lazy-creation logic which lets a cursor hold a domain with no replica DBs yet.