PDBStorage.write() replays a rolled-back transaction without any bound, and configuration changes hold an entry container's exclusive lock across it

[maximthomas]

Describe the bug

PDBStorage.WriteableStorageImpl.write() replays a rolled-back transaction, and nothing bounds
how many times:

// opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java:635
public void write(WriteOperation operation) throws Exception
{
  final Transaction txn = db.getTransaction();
  for (;;)
  {
    txn.begin();
    try
    {
      try
      {
        operation.run(this);
        txn.commit(commitPolicy);
        return;
      }
      catch (final StorageRuntimeException e)
      {
        throw unwrap(e);
      }
    }
    catch (final RollbackException e)
    {
      // retry after random sleep (reduces transactions collision. Drawback: increased latency)
      Thread.sleep((long) (Math.random() * MAX_SLEEP_ON_RETRY_MS));
    }
    ...

No attempt count, no wall-clock window, and the sleep is at most 50 ms. A caller of
Storage.write on PDB has no way to be told "this did not go through" — under sustained conflict
the call simply does not return.

PDB is the only engine here that is unbounded:

engine	bound on the replay
pdb	none — for (;;)
jdbc	MAX_RETRIES = 10 attempts and a 10 s MAX_RETRY_WINDOW_NANOS wall-clock window, with exponential backoff (JDBCStorage.java:946, :1030)
je	no replay at all — one transaction, the failure goes to the caller (JEStorage.java:913)
cassandra	no transaction, so nothing to replay (CASStorage.java:196)

Why it matters

Three callers hold an entry container's exclusive lock across a storage.write(...), so an
unbounded replay is an unbounded stall of everything else on that suffix, not just of the caller:

• EntryContainer.AttributeJEIndexCfgManager.applyConfigurationDelete — EntryContainer.java:248
• AttributeIndex.applyConfigurationChange — AttributeIndex.java:945 ("We get exclusive lock to
  ensure that no query is actually using the indexes that will be deleted")
• BackendImpl.applyConfigurationChange — added by [#907] Change the base DNs of a pluggable backend outside the write the storage replays #914, for the same reason: the trees of a
  removed base DN are now deleted while that base DN is still registered

EntryContainer.lock() sets exclusiveAccessPending, which parks every subsequent
beginSharedAccess() caller on sharedAccessMonitor.wait() with no timeout
(EntryContainer.java:2843, :414). So while the admin thread spins in the retry loop, every
worker thread that touches that suffix parks indefinitely, and dsconfig never returns either.
lock() also sleep-polls its drain uninterruptibly, so a long-running search on the suffix stalls
the configuration-change thread first.

This is not a regression from #914 — the first two call sites predate it — but #914 adds a third,
and it was raised in review there
(#914 (comment)).

What it needs

Bound PDBStorage.write() the way JDBCStorage.write() is already bounded: an attempt cap and a
wall-clock window, whichever comes first, then let the RollbackException reach the caller so the
configuration change reports a failure instead of never returning. The exponential backoff in
JDBCStorage.sleepBeforeRetry is the model; PDB's flat Math.random() * 50 ms is not.

Two things to settle when doing it:

1. The bound has to leave a genuinely contended write room to succeed. A conflict on PDB is a
   page-level write-write conflict, and the containers under the exclusive lock are precisely the
   ones no user write can reach — so the conflicts that drive this loop come from other base DNs
   in the same backend touching shared structures. Sizing the window against that, rather than
   against a single-suffix benchmark, is the part worth measuring.
2. ReplayedConfigChangeTest pins the current contract. aRemovalAndAnAdditionInOneChangeSurviveRepeatedReplay
   arms two conflicts and asserts attempts() == 3, on the stated grounds that "the contract is that
   the operation is replayed until it succeeds rather than that it survives a single replay". A cap
   has to stay clear of what that test arms, and the test comment needs rewording to say what the
   new contract is.

Related

• [#907] Change the base DNs of a pluggable backend outside the write the storage replays #914 — added the third call site, and where this was raised
• JDBC backend: nothing bounds an attempt of the transaction replay, so the retry window has to work around it #915 — the same question for the JDBC backend, from the other end: there the replay is bounded
  and it is an attempt that is not
• Pluggable backends: a replayed applyConfigurationChange fails on an already-deregistered base DN #907 — what [#907] Change the base DNs of a pluggable backend outside the write the storage replays #914 fixes