[#990] Drop what a previous index left behind instead of adopting it when an index is added

[vharseko]

Fixes #990.

The defect

The name of an index tree is a pure function of (base DN, attribute, index id)
(AttributeIndex.getIndexName), and open(txn, true) is open-or-create and never truncate, so an index
added for an attribute a previous index served reopens exactly the trees that one left behind — with
their content, and with the TRUSTED flag their state records still carry. DefaultIndex.afterOpen
reads that flag back, and applyConfigurationAdd says something only when the index comes back
untrusted, so the adoption is silent: searches answer out of trees which know nothing of the entries
written while no configuration named them.

A state record can also outlive its tree. On JDBC deleteTree drops the table with DDL which commits
of its own accord, while the state.deleteRecord of the same closeAndDelete belongs to the
transaction; a rollback in between restores the record over a table which is gone, and the index added
next is created empty and read back as trusted — every search on that attribute then answers with
nothing.

How the trees get there

• an index deleted while the backend is disabled, or through an offline dsconfig: the add and delete
  listeners live only while the entry container is open (EntryContainer constructor, close()), so
  nothing deletes the trees and no failure is involved;
• a deletion the storage gives up on (Config change paths do not flag an admin action when a bounded storage write gives up #962), which leaves the trees behind while the configuration entry
  is already persisted — ConfigurationHandler.deleteEntry writes the configuration before it notifies
  the listeners;
• a stop between the deletion of the trees and the commit of the configuration change.

The fix

The three paths which open an index the configuration is adding drop first whatever an index of the
same name left behind — the tree and the state record which goes with it — and only then create it:

Path	Where
index add listener	EntryContainer.AttributeIndexCfgManager.applyConfigurationAdd → AttributeIndex.dropLeftovers
VLV index add listener	EntryContainer.VLVIndexCfgManager.applyConfigurationAdd → VLVIndex.dropLeftovers
an index type declared again	AttributeIndex.applyConfigurationChange → dropLeftoversOf

The index is then where any other index added to a backend holding entries starts: empty, untrusted and
asking to be rebuilt through NOTE_INDEX_ADD_REQUIRES_REBUILD. Nothing is lost that a rebuild does not
regenerate.

Four things about how it is done:

• The drop and the open are two writes, the first committed before the second. On JE
  removeDatabase(txn, …) write-locks the record of the name in _jeNameMap until the transaction
  commits, and JEStorage.getOrOpenTree opens a tree with openDatabase(null, …) — a transaction of its
  own — which asks for a read lock on that record and, with lockTimeout=0, waits for it without limit.
  There is no cycle, so the deadlock detector is silent; the change never returns, and while it is
  parked it holds JEStorage.trees' monitor, behind which every tree-cache miss of the backend queues.
  A drop in a write of its own does not reach that. Failing between the two leaves the trees gone and
  the configuration already written — which the next open of the backend resolves by creating them
  empty and untrusted, the same outcome.
• The state record is taken out whether a tree was found or not. A probe for the trees alone would
  miss the JDBC window above, where the tree is gone and only the record is left — precisely the case
  which yields an empty trusted index. A record deleted on its own is not reported as discarded
  content, since none was: the index is untrusted and asks for its rebuild like any other.
• Marking the index untrusted would not have been enough. DefaultIndex.get returns what a key holds
  whenever it holds anything, whatever the flag says; only an absent key answers undefined and sends the
  search to the entries. The flag is consulted by IndexQueryFactoryImpl.readRange (range queries) and
  not by createExactMatchQuery, so an adopted equality, presence or substring tree would go on
  answering with another index's content. VLVIndex.evaluate does refuse an untrusted index, but its
  trees are dropped too, for one rule rather than two.
• The tree is asked for through the transaction, txn.treeExists(name), not through listTrees().
  On JDBC listTrees() borrows a connection of its own, which a transaction already holding one of the
  same pool must not ask for, while treeExists asks the transaction's own; on Cassandra listTrees()
  is a stub answering nothing, while treeExists finds the partition and deleteTree deletes it, so the
  fix holds there too; and a replayed attempt of the drop write then sees what is there when it runs.
  On JE getDatabaseNames() neither locks nor lists a name the same transaction has removed.

Only the trees of the index ids the new configuration declares are looked at. A tree of an id it does
not name (the old index had ordering, the new one has not) is opened by nothing and answers nothing,
and the first configuration which declares that id again drops it the same way; reporting it is part of
the orphan-tree follow-up below.

Nothing is dropped under the entry container's exclusive lock: an index which is only being added is in
no map a search reaches, and the trees it would have adopted are named by nothing until it opens them.
That holds only while no live index names them, and there was a way for one to: attrIndexMap is keyed
by the attribute type, which every one of the attribute's names and its OID resolve to
(AttributeTypePropertyDefinition.decodeValue), while the configuration entry is named by whichever of
them was typed and ConfigurationHandler.addEntry refuses only an identical DN. So
create-backend-index --index-name commonName over a live cn index was admitted, and would have
dropped the trees the live index serves as left behind. It is refused now, in
isConfigurationAddAcceptable — before the configuration is written — and again at the top of
applyConfigurationAdd, with ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED (629) naming the attribute,
the base DN and the configuration entry which already indexes it. At base the same add reopened the
existing trees and replaced the index in the map, harmless but wrong.

WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES (628) names the index and the base DN in the change result and
in the error log, so what a backend was left holding is findable after the session which changed it has
ended. Ids 624-627 are deliberately skipped: #994 is using them.

Tests

IndexAddedOverLeftoverTreesTest leaves trees behind the way a disabled backend does — it declares a cn
index and a VLV index, fills them with an entry, reopens the backend from a configuration which no
longer names them, and writes another entry which none of them sees. Every case runs over PDB and over
JE (a @DataProvider over the storage; JE is where the two writes sharing a transaction hang, so the JE
rows are what pins that). Six cases cover the defect and two pin what must not change:

• an index added over those trees is not trusted, asks for a rebuild and reports the discarded content;
• none of the trees it opens answers with a key the trees left behind held (read through
  MatchingRuleIndex.get, so this is what a search would get);
• an index added over a state record whose trees were deleted — the JDBC window, reproduced by deleting
  the trees and leaving the records — is not trusted, and reports no discarded content;
• a VLV index added over its trees is not trusted, its tree and its counter are emptied and the TRUSTED
  record is gone;
• an index type declared again through applyConfigurationChange drops the tree of that id only: the
  live equality tree keeps answering the key it held;
• a second index for an attribute type which is already indexed is refused, before and at the add, and
  the live index answers the keys it held before;
• an index added to an empty backend is still trusted and says nothing, and an index added to a non-empty
  backend with nothing left behind still asks for a rebuild and reports nothing discarded.

All of the defect cases fail on master for the reason each names, and pass here.

Run locally: the class (16/16, PDB and JE), and PDBTestCase, EncryptedPDBTestCase, PDBStorageTest,
JETestCase, EncryptedJETestCase, ReplayedConfigChangeTest, DefaultIndexTest, StateTest, OnDiskMergeImporterTest,
PersistentCompressedSchemaTest, RebuildIndexTestCase, VerifyIndexTestCase, ControlsTestCase — 315 tests, green.

Interlock with #994

#994 adds anIndexCreatedAgainAdoptsTheTreesAFailedDeletionLeftBehind, which pins the behaviour this PR
removes, and three message texts (ERR_CONFIG_INDEX_DELETE_FAILED 624, ERR_CONFIG_VLV_INDEX_DELETE_FAILED
625, ERR_CONFIG_INDEX_CHANGE_FAILED 626) which tell the operator that an index declared again adopts the
trees and is trusted over their content. Whichever lands second has to invert that test and correct those
three sentences — the trees are now discarded, and the index asks to be rebuilt.

Not in this change

• An index added while the backend is disabled, or through an offline dsconfig, over trees an
  index deleted the same way left behind: no listener runs, the configuration is written, and the next
  EntryContainer.open opens the trees with their TRUSTED record like any index the configuration has
  always named. Nothing at open time tells that index from one which was there all along — that needs a
  record of what the configuration named the last time the backend was open — so it is a follow-up
  issue, not a case here.
• Nothing yet reports a tree no configuration names when a backend is opened: no path compares
  storage.listTrees() with the configuration (BackendImpl.giveUpBaseDNsWhoseTreesAreGone does, for a
  different purpose). backendstat list-raw-dbs remains the way to find them. That is worth its own issue
  rather than this one.

[maximthomas]

praise: The three roads which reopen a tree now drop what they would have adopted, and the reasoning for it is written where the next reader needs it.

• listTrees() read before the write (EntryContainer.java:204-209, AttributeIndex.java:996-1001): the JDBC pool rule and the replay rule are both stated and both hold.
• index.close() before openAsAdded (EntryContainer.java:215-217) and the built.getAndSet(null) swap (:329-333): a replayed attempt leaves one listener, not one per attempt.
• The record-only road is healed, not ignored (AttributeIndex.dropLeftoversOf): the record is taken out whether a tree was found for it or not, and case 3 pins it with persistedFlags().
• VLVIndex.openAsAdded deletes the counter and the tree on their own (VLVIndex.java:194-203), which a half-done change and PersistIt's delete-of-absent both need.
• The test leaves real trees behind through a real configuration swap (leaveTreesBehind, :340-388) instead of mocking the storage; its content check (case 1, :163-172) reads the key the tree held, not a flag.

---

issue (blocking): On JE, adding an index over a leftover tree parks the config thread forever.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:471-478, :497-511; VLVIndex.java:194-209; opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java:523, :732

dropLeftoversOf → txn.deleteTree → JEStorage.deleteTree:523 env.removeDatabase(txn, …) takes the WRITE lock on the name record under the write's Txn. open(txn, true) in the same write → JEStorage.getOrOpenTree:732 env.openDatabase(null, …) runs under a separate auto-commit Txn, asks for the READ lock on the same record, and with lockTimeout=0 (ConfigurableEnvironment.java:359) waits forever; there is no cycle, so the deadlock detector is silent. While parked it holds Environment.openDatabase's monitor and JEStorage.trees' monitor, so every tree-cache miss on that backend blocks behind it. No result, no log line; only a kill ends it. Measured with a stand-alone JE 18.3.12 probe: remove(txn) then open(null) of one name — lockTimeout=0 blocks (5 s watchdog), lockTimeout=500 throws LockTimeoutException … database=_jeNameMap type=READ. PDB takes no name lock, which is why the PDB-only class is green. Same pair in VLVIndex.openAsAdded and in createIndex on the index-type road.

The drop and the open must not share a JE transaction: drop in a write of its own, open in the next.

// AttributeIndex: the drop half of openAsAdded, on its own
boolean dropLeftovers(WriteableTransaction txn, Set<TreeName> storedTrees)
{
  boolean dropped = false;
  for (Index index : indexIdToIndexes.values())
  {
    dropped |= dropLeftoversOf(txn, index, storedTrees);
  }
  return dropped;
}

// EntryContainer.AttributeIndexCfgManager.applyConfigurationAdd
storage.write(txn -> discarded.set(index.dropLeftovers(txn, storedTrees)));  // removeDatabase(txn) commits here
storage.write(txn -> {
  index.close();
  index.open(txn, true);                                                      // openDatabase(null, …) no longer waits
  trusted.set(index.isTrusted());
  attrIndexMap.put(cfg.getAttribute(), index);
  attrCryptoMap.put(cfg.getAttribute(), cryptoSuite);
});

Same split for VLVIndex.openAsAdded (trusted = false stays with the open half) and for the loop over addedIndexes in applyConfigurationChange (:1004-1014: one write dropping, one write opening).

Pin: a JE twin of LeftoverBackend (BackendImpl<JEBackendCfg> over JEStorage) running the same cases with @Test(timeOut = 60_000): at head case 1 hangs, with the split it passes.

---

issue (blocking): A second backend-index for an already indexed attribute, named by an alias or its OID, drops the live index.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java:183-195, :212-224; AttributeIndex.java:433, :497-511

The tree name is a function of the resolved type (attrType.getNameOrOID() + "." + indexID), the config key is what the operator typed. With cn indexed and live, an add of ds-cfg-attribute: commonName (or 2.5.4.3) is admitted: ConfigurationHandler.addEntry:453 rejects only an identical normalized DN, isConfigurationAddAcceptable only builds the object, and AttributeTypePropertyDefinition.decodeValue:82-97 resolves the alias to the cn type. openAsAdded then finds cn.equality in storedTrees, dropLeftoversOf deletes the live tree and its TRUSTED record outside EntryContainer.lock(), open(txn, true) recreates it empty, and attrIndexMap.put replaces the live index. At base the same add reopened the existing trees and changed nothing.

// EntryContainer.AttributeIndexCfgManager.isConfigurationAddAcceptable
if (attrIndexMap.containsKey(cfg.getAttribute()))   // keyed by the resolved type: cn, commonName and 2.5.4.3 are one key
{
  unacceptableReasons.add(LocalizableMessage.raw("Attribute %s of backend base DN %s is already indexed",
      cfg.getAttribute().getNameOrOID(), getBaseDN()));   // a proper ERR_ id: 629 is the next free ordinal
  return false;
}

Pin: a case adding a second BackendIndexCfg with ds-cfg-attribute: commonName over a live cn index; the add is refused and the key read by keysHeldBy(storage, cnTrees) before it still answers after it.

---

issue (non-blocking): The VLV road's tree drop and record deletion are pinned by nothing.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexAddedOverLeftoverTreesTest.java:223-244; VLVIndex.java:194-209

Case 4 asserts isTrusted() only, and openAsAdded sets trusted = false unconditionally (:209). Measured at head: both drop blocks deleted — 7/7 green; state.deleteRecord deleted — 7/7 green. Under the second mutant the TRUSTED record outlives the add and the constructor reads it back at the next open (:137): the JDBC-window defect on the VLV road, which has no case although the Tests paragraph says "the same for a VLV index".

// case 4, after applyConfigurationAdd
final Storage storage = backend.getRootContainer().getStorage();
assertThat(storage.<Boolean>read(txn -> {
      try (Cursor<ByteString, ByteString> cursor = txn.openCursor(backend.leftoverVlvTree)) { return cursor.next(); }
    })).as("the VLV tree left behind still holds its content").isFalse();        // kills "trees not dropped"
assertThat(persistedFlags(backend, backend.leftoverVlvTree))
    .as("the TRUSTED record outlived the add").doesNotContain(TRUSTED);           // kills "record not dropped"

Or: the VLV twin of case 3 (record left over a tree which is gone) for the restart road.

---

issue (non-blocking): The index-type road (case 5) pins only the flag.

IndexAddedOverLeftoverTreesTest.java:263-282; AttributeIndex.java:1063-1074

"deleteTree branch deleted" in dropLeftoversOf falls through to the record deletion, the index is untrusted, the case is green — with the substring tree still holding its leftover key. An over-broad drop taking the live equality tree is green too. And "state.deleteRecord deleted" alone is green in cases 1, 2 and 5, because EntryContainer.deleteTree:2477-2481 already removes the record on the tree-present road.

// case 5: read the keys before the change, then pin each tree on what only it holds
final Map<TreeName, ByteString> keysHeld = keysHeldBy(storage, backend.leftoverIndexTrees);   // before
…
final MatchingRuleIndex substring = index.getNameToIndexes().get("substring");
assertThat(storage.read(txn -> substring.get(txn, keysHeld.get(substring.getName())).isDefined()))
    .as("a substring key answered out of the tree left behind").isFalse();
final MatchingRuleIndex equality = index.getNameToIndexes().get("equality");
assertThat(storage.read(txn -> equality.get(txn, equalityKeyReadBefore).isDefined()))
    .as("the live equality tree went with the leftover").isTrue();

---

issue (non-blocking): WARN 628 reports discarded content where no tree was dropped.

AttributeIndex.java:497-511, :514-519; VLVIndex.java:205; opendj-server-legacy/src/messages/org/opends/messages/backend.properties:1135-1139; opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java:624-627

dropLeftoversOf returns true for a record deleted alone, so the JDBC record-only window reports "trees … content has been discarded" with no tree in sight. On Cassandra it is every add: CASStorage.listTrees() is a TODO stub returning emptySet(), so no tree is ever a leftover, the content is adopted untrusted — the behaviour the description says is replaced — and the WARN fires whenever a record existed.

private boolean dropLeftoversOf(WriteableTransaction txn, Index index, Set<TreeName> storedTrees)
{
  if (storedTrees.contains(index.getName()))
  {
    entryContainer.deleteTree(txn, index);   // the record goes with the tree
    return true;
  }
  state.deleteRecord(txn, index.getName());  // record only: nothing was discarded
  return false;
}

Same in VLVIndex.openAsAdded (state.deleteRecord(txn, getName()); without the |=). And say in the description that on Cassandra the drop is a no-op until listTrees() is implemented.

---

issue (non-blocking): WARN 628 is asserted by no case.

IndexAddedOverLeftoverTreesTest.java:140, :224, :250, :282, :333; AttributeIndex.java:514-519

Every message assertion is contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal()); "WARN not added" and "reportDiscardedLeftovers never called" are green 7/7. The Tests paragraph says the WARN is "in the change result and the error log".

assertThat(ordinalsOf(ccr)).contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal());       // cases 1, 2, 4, 5
assertThat(ordinalsOf(ccr)).doesNotContain(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); // case 7: nothing left behind

---

suggestion (non-blocking): createIndex reports inside the write the listener roads report outside of.

AttributeIndex.java:1063-1068, :1004-1014; EntryContainer.java:225-231, :339-344

On the index-type road reportDiscardedLeftovers runs inside the replayed write, so a replayed attempt repeats the WARN in the result and the log — the reason the two listener roads moved it out. Reachable on the record-only road only (a DROP or CREATE sets partlyCommitted, JDBCStorage:3111 refuses the replay), and the NOTE sibling was already inside at base.

final Set<Index> discarded = ConcurrentHashMap.newKeySet();
storage.write(txn -> {
  for (MatchingRuleIndex addedIndex : addedIndexes.values())
  {
    if (dropLeftoversOf(txn, addedIndex, storedTrees)) { discarded.add(addedIndex); }
    addedIndex.open(txn, true);
  }
});
for (Index index : discarded) { reportDiscardedLeftovers(ccr, index.getName(), entryContainer.getBaseDN()); }

(with the drop in a write of its own per the first issue).

---

question (non-blocking): Is the disabled-add road out of scope on purpose?

EntryContainer.java:550-556, :570

All online, no restart: disable the backend → delete-backend-index cn (no listener: the entry goes, the trees stay) → enable → write an entry with cn → disable → create-backend-index cn (no listener: the entry is persisted) → enable. EntryContainer.open runs index.open(txn, shouldCreate), adopts the stale tree and reads its TRUSTED record: (cn=…) misses the entry, silently — worse than the outcome this PR gives the online add. The description names this road neither as covered nor as excluded; if it is a follow-up, say so there.

---

suggestion (non-blocking): Only the ids the new configuration declares are dropped.

AttributeIndex.java:451-478

openAsAdded iterates the new indexIdToIndexes; a tree of an id the new configuration does not name (the old index had ordering, the new one equality only) is neither dropped nor reported and stays on disk until some later configuration declares it. Either narrow the javadoc ("whatever an index of the same name left behind") to the declared ids, or drop by prefix:

final String prefix = attrType.getNameOrOID() + ".";
for (TreeName stored : storedTrees)
{
  if (stored.getBaseDN().equals(entryContainer.getBaseDN()) && stored.getIndexId().startsWith(prefix)
      && !indexIdToIndexes.containsKey(stored.getIndexId().substring(prefix.length())))
  {
    txn.deleteTree(stored);
    state.deleteRecord(txn, stored);
    dropped = true;
  }
}

---

issue (non-blocking): The openBackend() catch NPEs on the road it was written for.

IndexAddedOverLeftoverTreesTest.java:483-506

rootContainerMonitor is assigned at BackendImpl.openBackend:234, after every point the comment names; the catch's finalizeBackend() → closeBackend:259 deregisterMonitorProvider(null) → DirectoryServer:2750 NPE before rootContainer.close() at :269. The volume stays open, the next case's removeStorageFiles() runs against it, and the original failure is buried under the NPE.

catch (Exception e)
{
  try
  {
    final RootContainer root = backend.getRootContainer();
    if (root != null) { root.close(); } else { backend.storage.close(); }   // finalizeBackend() NPEs before the monitor is registered
  }
  catch (Exception cleanupFailure) { e.addSuppressed(cleanupFailure); }
  throw e;
}

[vharseko]

Round 2 is pushed (8fbb0ed509), rebased onto master. Every point is taken; one goes further than asked, and is called out.

JE parks the config thread (blocking). Reproduced first with a stand-alone JE 18.3.12 probe: removeDatabase(txn, name) then openDatabase(null, name) on the same thread blocks at lockTimeout=0 and throws LockTimeoutException … database=_jeNameMap type=READ (owner: the txn's WRITE) at 500 ms — as you measured. All three roads now drop in a write of their own and open in the next: AttributeIndex.dropLeftovers / dropLeftoversOf no longer open anything, EntryContainer runs two writes on both add roads, and applyConfigurationChange runs a drop write and an open write over addedIndexes. VLVIndex.openAsAdded became the static VLVIndex.dropLeftovers(txn, entryContainer, state, name): with the record gone before the instance is built, the constructor reads no TRUSTED flag, and the trusted = false override went with it. Pinned by running the whole class over JE as well as PDB (@DataProvider over the storage; JEStorage's constructor is public now, as PDBStorage's is). A/B: round-1 main code under the round-2 test — the JE row of case 1 sits until the test timeout, the PDB row passes; with the split all 16 pass.

The tree is now asked for with txn.treeExists(name) inside the drop write, not with listTrees() read before it — a departure from a point you praised, so the reasons: once the drop is a write of its own, the replay argument for the snapshot no longer applies (that write creates nothing, so an attempt sees what the change found); on JDBC treeExists goes through the transaction's own connection, which is what the snapshot was avoiding the pool for; and on Cassandra listTrees() is a stub returning nothing while treeExists (a LIMIT 1 on the partition) and deleteTree are real, so the fix holds there instead of being a silent no-op. On JE getDatabaseNames() takes no lock and already hides a name the same transaction removed — probed too, since the second index id of a drop write asks after the first was removed.

A second index for an indexed attribute (blocking). Refused in isConfigurationAddAcceptable and again at the top of applyConfigurationAdd, since that is where the trees would go: ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED (629) names the attribute, the base DN and the configuration entry which already indexes it (AttributeIndex.getConfiguration().dn()). Pinned by aSecondIndexForAnAttributeAlreadyIndexedIsRefused: both refusals, the live index still the same instance, trusted, and answering every key it held.

VLV road pinned by nothing / case 5 pins only the flag / WARN asserted by no case. Case 4 now asserts the VLV tree and its counter hold nothing after the add and the record carries no TRUSTED; case 5 reads the keys before the change and pins the substring key gone and the equality key still answered; the WARN ordinal is asserted in cases 1, 2, 4, 5 and its absence in 3 (record only) and 7 (nothing left behind).

WARN where no tree was dropped. dropLeftoversOf returns true for a tree only; the record is deleted either way and reported by nothing, the index asks for its rebuild through the NOTE as before. Case 3 pins the absence. The Cassandra paragraph is in the description — with treeExists it is no longer a no-op there.

createIndex reports inside the write. Gone: the drop write and the open write each collect what they found, and both the WARN and the NOTE are reported after them — the NOTE too, since the write was being restructured anyway.

Disabled-add road. Out of scope on purpose, now said so under "Not in this change": an index added while the backend is disabled over trees deleted the same way is opened by EntryContainer.open like any index the configuration always named, and nothing at open time tells the two apart without a record of what the configuration named last time. Follow-up issue.

Only declared ids. Javadoc narrowed rather than dropping by prefix: a tree of an undeclared id is opened by nothing and answers nothing, the first configuration which declares the id drops it the same way, and a prefix drop over attrType.getNameOrOID() + "." would need its own rules for attributes named by OID. Left to the orphan-tree follow-up with the reporting.

openBackend() catch. Taken as proposed: root.close(), or the storage when there is no root.

Run locally: the class 16/16 (PDB and JE) and PDBTestCase, EncryptedPDBTestCase, PDBStorageTest, JETestCase, EncryptedJETestCase, ReplayedConfigChangeTest, DefaultIndexTest, StateTest, OnDiskMergeImporterTest, PersistentCompressedSchemaTest, RebuildIndexTestCase, VerifyIndexTestCase, ControlsTestCase — 315 tests, green.