[#991] Decide and report an index configuration change outside the write which is replayed

[vharseko]

Fixes #991.

AttributeIndex

applyConfigurationChange called createIndex and updateIndex from inside a WriteOperation, and both wrote into the caller's ConfigChangeResult and mutated in-memory state no rollback undoes. Storage.write() may replay a rolled back operation — PDBStorage and JDBCStorage both do — so:

• every replayed attempt added its messages again, and the operator was told to rebuild an index once per attempt;
• setIndexEntryLimit() answers "the limit went up" by comparing the new limit with a field it then assigns, so the replay compared the raised limit with itself, found nothing to rebuild, and committed an index whose entry limit was raised while the TRUSTED flag the rollback had put back was still stored. DefaultIndex.afterOpen reads that flag back after a restart and the index is used as if it were complete.

Now what the indexes which stay are asked for is decided before any of the three writes — updateIndex becomes planIndexUpdates, which reads getIndexEntryLimit() rather than the return value of the setter — and reported there as well, because the instruction holds whichever way the writes go: ConfigurationHandler.replaceEntry has already written the raised limit to config.ldif when the listener runs, and the next open of the index applies it to a tree whose keys were given up under the lower one. A write the storage gives up on therefore leaves the rebuild instruction in the result next to the failure, where the first round dropped it together with the limit. The untrust write does only setTrusted(txn, false), and only when there is something to untrust — a lowered limit opens no transaction, as VLVIndex already did not; the entry limit is applied once that write has committed. createIndex answers whether the index it opened has to be rebuilt instead of reporting it, and the report follows the write, as EntryContainer.applyConfigurationAdd:204-225 already does for the index it adds and for the same reason: that answer needs the transaction.

VLVIndex

VLVIndex.applyConfigurationChange had the same shape and is included here rather than tracked on its own, since the analysis and the fix are one and the same. Every question it asked was asked of the configuration it holds, and both that configuration and the four fields around it were assigned inside the write, so a replay finds nothing changed.

Worth stating precisely, because it is not what one would expect: the replay does still remove the flag today, and what keeps it doing so is that the rolled back attempt left adminActionRequired in the shared ConfigChangeResult — the same accident that repeats the message. The observable defect is therefore the duplicated message; the persisted flag rides on that duplication. Decided and reported before the write and published after it, neither depends on the other any more.

A conflict raised by the flag removal itself was also caught inside the operation and turned into a message, so the storage was never told to replay it and committed an attempt which had done nothing. It now reaches the retry loop. What the storage gives up on is reported the way AttributeIndex reports it — server error, the stack trace as a message — with the result built so far, rather than thrown: ConfigurationHandler catches nothing a listener throws and would discard the rebuild the result asks for together with the exception.

Tests

Eight tests in ReplayedConfigChangeTest, which already carries the replaying storage from #907:

• anIndexAddedByAChangeIsReportedOnceWhenTheTransactionIsReplayed
• aRaisedEntryLimitUntrustsTheIndexWhenTheTransactionIsReplayed
• aRaisedEntryLimitIsReportedWhenTheWriteWhichUntrustsTheIndexGivesUp — and re-applies the change after the give-up, which is a change again because the plan asks the index, not the configuration the failed change had already put in place
• aRaisedEntryLimitIsReportedWhenTheWriteWhichAddsIndexesGivesUp — the same instruction when the first write gives up
• aLoweredEntryLimitNeedsNoRebuildAndOpensNoTransaction
• aChangedSortOrderUntrustsTheVlvIndexWhenTheTransactionIsReplayed
• aChangedSortOrderIsReportedWhenTheWriteWhichUntrustsTheVlvIndexGivesUp
• aConflictRaisedByTheRemovalOfTheVlvIndexFlagIsReplayed

The harness gains conflictAtCommitOnWrite(nth, conflicts) and failWithoutReplayOnWrite(nth), since an index change makes three writes and each test has to arm the one it is about; conflictAsTheTransactionReportsIt(conflicts), which raises the first-access conflict in the form PDBStorage's own transaction raises it — a StorageRuntimeException wrapping the RollbackException, the only form an inner catch of the former ever met; and a vlvIndex to open where a test asks for one. VLVIndex gains a package-private getSortKeys(), so that the definition a committed write published, and the one a failed write did not, can be read.

Red at the base for the assertion each is about: two messages instead of one; [TRUSTED, COMPACTED] still stored after the raised limit; the vlvIndex conflict swallowed (one attempt, server error, flag intact); the give-up roads answered with the limit an attempt which rolled back had assigned, or with an exception in place of a result. The pins added in the third round are each red under the mutant they are about: the report moved back before the third write, the plan asking the configuration instead of the index, the vlvIndex definition unpublished or published before the write, the give-up catches without the failure. The fourth round's pin is red under the same hoist on baseDN, scope or filter: the vlvIndex give-up case now changes all four published fields at once, read back through getBaseDN(), getScope() and getFilter() next to getSortKeys(). Green here: ReplayedConfigChangeTest 20/20, and PDBTestCase, DefaultIndexTest, BulkCursorTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase — 134 tests, no failures.

Not in this change

index-entry-limit 0 means no limit, and the comparison planIndexUpdates inherits from DefaultIndex.setIndexEntryLimit classifies it as the smallest limit. Pre-existing; filed as #1059 rather than folded in, since the method is already shared by #994 and #1000 in flight.

The confidentiality arm of planIndexUpdates is reached by no case here. It cannot be reached by the fixture's equality index: enabling confidentiality on it changes the index ID to equality:hash, which makes the change an addition and a removal rather than an update. #1000 replaces that arm — the decision moves to a comparison of the two configurations, and the writes change — so its pin belongs there.

Note for the merge

#994 restructures the same method for #962 — it moves the publication of config, indexingOptions and indexIdToIndexes around the same three writes — and #1000 restructures it again for #992, taking setConfidential out of Index, which planIndexUpdates calls. The three conflict textually and whichever land later will need the rebase.

[maximthomas]

praise: The change puts the decision and the report outside the operation the storage may replay, which is the shape #991 needs.

• planIndexUpdates (AttributeIndex.java:1038) decides from the pre-change state, so a replayed untrust write reaches the same answer on every attempt.
• The inner catch is gone from VLVIndex.java:281-293: a conflict inside the untrust operation now reaches the storage's retry loop — on PDB and on JDBC — instead of committing an attempt which did nothing.
• conflictAtCommitOnWrite (ReplayedConfigChangeTest.java:924) makes a change with several writes testable, and the three new cases are red at the base for the right assertion each.

---

issue (blocking): The rebuild instruction is decided before the write and delivered only if the write commits.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1006-1016, :942-953
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:290-292, :312-316

When the storage gives the write up (PDB after its retry budget, #937; JDBC after MAX_RETRIES), catch (Exception) at :1012 adds the server error and a stack trace; rebuildMessages + setAdminActionRequired(true) (:1006-1010) and the limit loop (:1002-1005) are skipped. What that road leaves: the index untrusted in memory (DefaultIndex.setTrusted :324 assigns before the tree write), TRUSTED intact on disk, and config.ldif already holding the raised limit — ConfigurationHandler.replaceEntry writes it at :642, before the listener loop, and nothing rolls it back. The log line says adminActionRequired=false, the client gets ERR_CONFIG_FILE_MODIFY_APPLY_FAILED with a stack trace naming no index, and the next restart trusts the index under the raised limit: the #991 end state, on a road where the base delivered NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD and adminActionRequired=true. Write 1 has the same shape for the added indexes. In VLVIndex the :290 catch rethrows, so the result carrying adminActionRequired=true (:248/:253/:263) is discarded whole.

The instruction is right on both roads, so say it before the write; only the limit belongs after the commit:

planIndexUpdates(updatedIndexes.values(), newConfiguration, indexesToUntrust, rebuildMessages);
for (LocalizableMessage rebuildMessage : rebuildMessages)
{
  ccr.setAdminActionRequired(true);
  ccr.addMessage(rebuildMessage);
}
entryContainer.getRootContainer().getStorage().write(/* untrust only, as now */);
for (final Index updatedIndex : updatedIndexes.values())
{
  updatedIndex.setIndexEntryLimit(newConfiguration.getIndexEntryLimit());
}

For VLVIndex, the :290 catch can report the way AttributeIndex :1012 does instead of throwing:

catch (final Exception e)
{
  ccr.addMessage(NOTE_INDEX_ADD_REQUIRES_REBUILD.get(getName()));
  ccr.addMessage(LocalizableMessage.raw(StaticUtils.stackTraceToSingleLineString(e)));
  ccr.setResultCode(DirectoryServer.getCoreConfigManager().getServerErrorResultCode());
  return ccr;
}

Pin: the NO_REPLAY twin of conflictAtCommitOnWrite in the harness, and one case per class on the failed road — red at the head (adminActionRequired false, no ordinal), green with the fix:

void failWithoutReplayOnWrite(int nth)
{
  arm(ConflictPoint.NO_REPLAY, 1);
  armedWrite = writes + nth;
}

backend.storage.failWithoutReplayOnWrite(3);
final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));
assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS);
assertThat(ccr.adminActionRequired()).as("the rebuild the raised limit needs, on the road which failed").isTrue();
assertThat(ordinalsOf(ccr)).contains(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).as("what the restart will read").contains(TRUSTED);

---

suggestion (non-blocking): The untrust write in AttributeIndex opens a transaction with nothing to do.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:991

storage.write runs with an empty indexesToUntrust — a lowered limit, a confidentiality change which needs nothing — where VLVIndex.java:271 guards it and says why: a bounded storage can give that write up with nothing to give up, which puts the blocking issue's road in front of a change that asks for nothing. Pre-existing, but the PR aligns the two classes.

if (!indexesToUntrust.isEmpty())
{
  entryContainer.getRootContainer().getStorage().write(...);
}

---

suggestion (non-blocking): No case lowers or keeps the entry limit, so < → != at the comparison survives every case.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1044, ReplayedConfigChangeTest.java:616-650

Every AttributeIndex case raises the limit (4000 → 8000). The mutant untrusts and reports on a lowered limit and nobody is red. A pre-existing gap, re-homed by the PR with the new cases around it.

final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 2000));
assertThat(ccr.adminActionRequired()).as("a lowered limit needs no rebuild").isFalse();
assertThat(ccr.getMessages()).isEmpty();
assertThat(cnIndex.isTrusted()).isTrue();
assertThat(cnIndex.getIndexEntryLimit()).isEqualTo(2000);

---

suggestion (non-blocking): "A change asking for nothing opens no transaction" is asserted on the result, not on the storage.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:271, ReplayedConfigChangeTest.java:680-683

The again call asserts SUCCESS, no admin action, no messages; delete the if (requiresRebuild) guard and it stays green. writes() (:955) is what pins it, as aChangeWhichLeavesTheBaseDNsAloneOpensNoTransaction already does at :542-547.

final int writesBefore = backend.storage.writes();
final ConfigChangeResult again = vlvIndex.applyConfigurationChange(vlvIndexCfg("+sn"));
assertThat(backend.storage.writes()).as("a change which changes nothing opens no transaction").isEqualTo(writesBefore);

---

suggestion (non-blocking): The raised-limit case arms the untrust write by its ordinal.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:629

conflictAtCommitOnWrite(3, 1) names the third write, and the comment above it explains why it is the third. #994 restructures the same method; once a write is merged or dropped the arm lands on another write and the case stays green pinning something else. The cheapest guard turns that red:

final int writesBefore = backend.storage.writes();
backend.storage.conflictAtCommitOnWrite(3, 1);
// ...
assertThat(backend.storage.writes()).as("the armed write was the last of three").isEqualTo(writesBefore + 3);

---

note (non-blocking): A failure of the untrust operation now leaves VLVIndex.applyConfigurationChange as an exception, and no case pins the road the PR claims.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:271-296

Stated as deliberate at :274-279, recorded rather than argued: ConfigurationHandler.replaceEntry (:647-656) catches nothing, so on the give-up road and on any non-conflict failure the client gets the worker's uncaught-exception response instead of ERR_CONFIG_FILE_MODIFY_APPLY_FAILED. The blocking fix above closes the give-up half. What stays unpinned is the claim itself — "the conflict now reaches the retry loop": conflictAtFirstStorageAccess throws a raw RollbackException, nothing throws the StorageRuntimeException State.removeFlagsFromIndex produces from inside the operation. A harness point which does, plus attempts() == 2 and the flag gone, pins it:

writeOperation.run(txn);
throw new StorageRuntimeException(new RollbackException());

---

note (non-blocking): index-entry-limit 0 means no limit, and < classifies it as the smallest limit.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1044 (mirrors DefaultIndex.java:304; DefaultIndex.java:247 is where 0 means unlimited)

4000 → 0 answers "no rebuild" though every key over 4000 is undefined and stays so; 0 → 4000 answers "rebuild" though nothing needs one. Pre-existing — the PR copies the expression into planIndexUpdates — so a follow-up is fine; the lowered-limit case above is the case it needs.

final int oldLimit = updatedIndex.getIndexEntryLimit();
final int newLimit = newConfig.getIndexEntryLimit();
final boolean newLimitRequiresRebuild = oldLimit != 0 && (newLimit == 0 || oldLimit < newLimit);

[vharseko]

Round 2 pushed as b7094cd, on top of a rebase to current master 129fc4e (range-diff of the first commit is =).

issue (blocking) — the rebuild instruction was delivered only if the write committed. Verified as described: any failure of a write — the bound spent, or a failure the engine does not replay at all (PDBStorage.write:692-696 on the first attempt) — landed in the catch at the end of the method, and ConfigurationHandler.replaceEntry:642 has already written the raised limit to config.ldif when the listener runs. Fixed, with one difference from the snippet: planIndexUpdates and the report now sit before the first write, not between the second and the third. The plan reads nothing the writes change (getIndexEntryLimit(); setConfidential only compares), and a give-up on the second write — the deletion — leaves the same end state after a restart as a give-up on the third, so the instruction has to survive that road as well. The limit itself is still applied after the untrust write has committed.

On "write 1 has the same shape": the answer there needs the transaction (open plus the flags), so it cannot precede the write. That road is not silent, though: the added index has no persisted flags, so the next open of the container logs NOTE_INDEX_ADD_REQUIRES_REBUILD for it (EntryContainer.open:537-539). Left as it is.

VLVIndex: the catch now reports the way AttributeIndex does — server error, the stack trace as a message — and returns the result it had built, so the rebuild it asked for and adminActionRequired reach ConfigurationHandler rather than being discarded with the exception. The message moved before the write too, so it is added in one place on both roads. The comment stating the opposite is gone.

Pins: failWithoutReplayOnWrite(nth) in the harness, aRaisedEntryLimitIsReportedWhenTheWriteWhichUntrustsTheIndexGivesUp and aChangedSortOrderIsReportedWhenTheWriteWhichUntrustsTheVlvIndexGivesUp. Both red at the previous head (adminActionRequired false and no ordinal; for the vlvIndex, an exception in place of a result), green now. The AttributeIndex one also pins the limit the failed write did not apply (still 4000); the vlvIndex one that the definition the failed change did not publish is still a change when asked for again.

suggestion — the untrust write with nothing to do. Guarded, third write only: if (!indexesToUntrust.isEmpty()). Writes 1 and 2 have the same shape with empty maps and are left alone here.

suggestion — no case lowers the limit. aLoweredEntryLimitNeedsNoRebuildAndOpensNoTransaction: 4000 → 2000, no admin action, no messages, still trusted, limit 2000, and writes() is before + 2 — which is also what pins the guard above.

suggestion — the again call asserted on the result only. writes() before and after, as in aChangeWhichLeavesTheBaseDNsAloneOpensNoTransaction.

suggestion — the arm by ordinal. writes() == before + 3 next to conflictAtCommitOnWrite(3, 1); the comment now says the third write is the one which removes the flag rather than the one which applies the limit.

note — "the conflict now reaches the retry loop" was unpinned. Agreed, and pinned — with a different harness point than the one in the note, because that one does not tell the base from the head. A throw placed after writeOperation.run(txn) lies outside the inner catch the base had, so the base replays it as well: attempts() == 2 and the flag gone are green there too, and only the message count is red, which conflictAtCommit(1) already pins. The inner catch bit what PDBStorage's own transaction raises from inside state.removeFlagsFromIndex — a StorageRuntimeException wrapping the RollbackException (PDBStorage.java:445-447), which ConflictingTransaction never raised. So the harness gains conflictAsTheTransactionReportsIt(n): the same first-access conflict in that form. aConflictRaisedByTheRemovalOfTheVlvIndexFlagIsReplayed is red at the base (one attempt, server error, TRUSTED intact) and green here (two attempts, SUCCESS, one message, flag gone). The bare form stays as it was for the existing cases.

note — index-entry-limit 0. Pre-existing in DefaultIndex.setIndexEntryLimit:304 and copied as is; filed as #1059 rather than folded in, since AttributeIndex.applyConfigurationChange is already shared by #994 and #1000 in flight. The lowered-limit case above is the one it will extend.

Runs: ReplayedConfigChangeTest 19/19; PDBTestCase, DefaultIndexTest, BulkCursorTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase — 133 tests, no failures. Against the previous head, the two give-up cases and the lowered-limit case are red; against master, those and the conflict case as well, each on its own assertion.

[maximthomas]

praise: Round 1's blocking issue is closed the way it should be: the decision and the report now precede every write, and the give-up road is executed, not reasoned about.

• ReplayedConfigChangeTest with both round-2 hunks of AttributeIndex.java and VLVIndex.java reverted: exactly the three new cases red, each on its own assert (:756 adminActionRequired, :815 an exception in place of a result, :790 before+3 writes), the other 16 green — 19/19 at head.
• failWithoutReplayOnWrite(nth) fails through the delegate's own classification (attempts()==1, the flag removal rolled back on disk), so the give-up cases pin what PDB does, not what the harness does.
• VLVIndex.applyConfigurationChange keeps the old definition on the give-up road and the again block (:824-828) pins that a re-apply still sees a change.
• The third write is opened only when there is something to untrust (:1004), pinned by writes()==before+2 in the lowered-limit case.

---

suggestion (non-blocking): "Before any of the three writes" is pinned on the write-3 road only.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:741-761, opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:938-947

Moving the plan-and-report block back to just before if (!indexesToUntrust.isEmpty()) (:1004) is green on all 19 cases: no case combines a raised limit with a give-up on write 1 or 2. Under that mutant a 4000→8000 change whose (empty) add write gives up is round 1's shape again — config entry rewritten, index TRUSTED at 4000, no instruction in the result.

@Test
public void aRaisedEntryLimitIsReportedWhenTheWriteWhichAddsIndexesGivesUp() throws Exception
{
  // Same fixture as aRaisedEntryLimitIsReportedWhenTheWriteWhichUntrustsTheIndexGivesUp, armed on the first write.
  backend.storage.failWithoutReplayOnWrite(1);
  final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));

  assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS);
  assertThat(ccr.adminActionRequired()).as("the rebuild, asked for before the first write").isTrue();
  assertThat(ordinalsOf(ccr)).contains(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
  assertThat(cnIndex.getIndexEntryLimit()).isEqualTo(4000);
  assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).contains(TRUSTED);
}

Pin: kills "report moved back before write 3" and pins the description's claim.

---

suggestion (non-blocking): The AttributeIndex give-up has no again twin; a re-apply after it is unpinned.

opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:741-761, opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:976, :1056

this.config = newConfiguration (:976) lands before write 3, so after a write-3 give-up config already holds 8000 while the live index holds 4000. The head re-detects because planIndexUpdates asks the live index (:1056); "compare config.getIndexEntryLimit() instead" is green on every case, and on a real re-apply it would apply 8000 to a trusted index with no report. VLVIndex has this twin (:824-828); the attribute index does not.

// After the asserts of aRaisedEntryLimitIsReportedWhenTheWriteWhichUntrustsTheIndexGivesUp:
final ConfigChangeResult again = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));
assertThat(again.getResultCode()).isEqualTo(ResultCode.SUCCESS);
assertThat(again.adminActionRequired()).as("the limit the failed write did not apply is still a change").isTrue();
assertThat(ordinalsOf(again)).contains(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
assertThat(cnIndex.getIndexEntryLimit()).isEqualTo(8000);
assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).doesNotContain(TRUSTED);

---

suggestion (non-blocking): The four VLV definition publications moved behind the write are read by no case.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:309-323

The move is the fix (old definition kept on give-up), but only this.config is observed, through again. this.sortKeys = newSortKeys deleted is green on the three VLV cases, and so is putting the four assignments back at detection time as on the base: VLVIndex has no getters, sortKeys is read only in key encoding (:455, :835) and evaluate (:524). After a successful change the old keys would then serve an in-JVM rebuild.

// VLVIndex — for the test
List<SortKey> getSortKeys()
{
  return sortKeys;
}

// aChangedSortOrderUntrustsTheVlvIndexWhenTheTransactionIsReplayed, after the flag asserts:
assertThat(vlvIndex.getSortKeys()).as("the definition the committed write published")
    .containsExactly(new SortKey("sn", false));
// aChangedSortOrderIsReportedWhenTheWriteWhichUntrustsTheVlvIndexGivesUp, before `again`:
assertThat(vlvIndex.getSortKeys()).as("the definition the failed write did not publish")
    .doesNotContain(new SortKey("sn", false));

Or: rebuild the VLV index after the replayed case and evaluate() a search sorted +sn — non-null — and one sorted the old way — null (:524).

---

suggestion (non-blocking): The confidentiality arm of planIndexUpdates is reached by no case.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1061-1069

The cfg mock never stubs isConfidentialityEnabled() and the test crypto suite is not encrypted, so setConfidential(...) is false in every case; deleting the arm is green on all 19. Base code (129fc4e3:1013), unpinned there too — the PR only gave it a new home.

@Test
public void aConfidentialityChangeUntrustsTheIndexWhenTheTransactionIsReplayed() throws Exception
{
  final BackendIndexCfg cfg = indexCfg(newTreeSet(IndexType.EQUALITY), 4000);
  when(cfg.isConfidentialityEnabled()).thenReturn(true);

  backend.storage.conflictAtCommitOnWrite(3, 1);
  final ConfigChangeResult ccr = index.applyConfigurationChange(cfg);

  assertThat(backend.storage.attempts()).isEqualTo(2);
  assertThat(ccr.adminActionRequired()).isTrue();
  assertThat(ordinalsOf(ccr)).containsOnly(NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD.ordinal());
  assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).doesNotContain(TRUSTED);
}

---

suggestion (if-minor): The give-up catches are pinned by contains(ordinal) and isNotEqualTo(SUCCESS) only.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:303-305, opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:755-757, :816-820

Deleting ccr.addMessage(LocalizableMessage.raw(stackTraceToSingleLineString(e))), or returning any other non-SUCCESS code, is green; what the operator reads on the give-up road is exactly that code and that cause.

assertThat(ccr.getResultCode()).isEqualTo(DirectoryServer.getCoreConfigManager().getServerErrorResultCode());
assertThat(ccr.getMessages().toString()).as("the failure, next to the rebuild").contains(UnreplayableFailure.class.getSimpleName());

---

question (non-blocking): JEStorage.write has no retry loop — is a JE issue wanted?

opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java:913-933

Single attempt, no RollbackException catch — same on the base, so nothing this PR changes: on JE a conflicting untrust write is a first-attempt give-up, and the report decided before the write is delivered on that road too. Worth an issue for a JE retry loop like PDB's, or is JE meant to give up first time?

[vharseko]

Round 3 pushed as 97c8773, on top of b7094cd. Test-only, but for one package-private getter on VLVIndex.

suggestion — "before any of the three writes" pinned on the write-3 road only. Verified: the report block moved back to just before if (!indexesToUntrust.isEmpty()) is green on all 19. aRaisedEntryLimitIsReportedWhenTheWriteWhichAddsIndexesGivesUp arms failWithoutReplayOnWrite(1): the instruction in the result, the limit still 4000, TRUSTED still stored, and writes() == before + 1 so that the arm cannot drift onto another write. Red under that mutant at "asked for before the first write", green at head.

suggestion — no again twin for the AttributeIndex give-up. Verified: config = newConfiguration (:976) precedes write 3, and planIndexUpdates taking the limit from that configuration instead of the index is green on every case. The give-up case now re-applies 4000 → 8000: SUCCESS, the rebuild asked for, the limit applied, TRUSTED gone. Red under that mutant at "still a change", green at head.

suggestion — the four VLV publications are read by no case. Verified both ways: this.sortKeys = newSortKeys deleted, and the four assignments back before the write with this.config left where it is, are each green on the three VLV cases. VLVIndex.getSortKeys() is added, package-private, and the replayed case pins [sn] after the commit while the give-up case pins [cn] after the failure and [sn] after the re-apply. The first mutant is red in both cases, the second in the give-up case.

suggestion — the confidentiality arm of planIndexUpdates is reached by no case. The arm is unpinned, but not by the case as written, which does not reach it: with EQUALITY and isConfidentialityEnabled() == true, buildBaseIndexers(protectIndexKeys = true) wraps the indexer in a HashedKeyEqualityIndexer whose ID is equality:hash (AttributeIndex.java:232, :359), and buildIndexesForIndexers keys the map by that ID (:390). updatedIndexes is then empty and the change is an addition of equality:hash plus a removal of equality: no third write, attempts() == 0, and NOTE_INDEX_ADD_REQUIRES_REBUILD in the result rather than NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD. The arm is reached only by an index whose ID the setting does not change — presence, substring, ordering, approximate — which the fixture does not hold. And #1000 (#992) takes setConfidential out of Index, decides confidentiality by comparing the two configurations before the writes, and adds writes of its own, so the ordinal the case would arm moves with it. Left to #1000, where enablingConfidentialityEmptiesTheIndexItAsksToRebuild already pins the report; both findings — the equality:hash road and the comparison against the configuration this attribute index holds, which is the re-apply hazard above — go there as review notes.

suggestion (if-minor) — the give-up catches pinned by contains(ordinal) and isNotEqualTo(SUCCESS). Both give-up cases and the new one assert getServerErrorResultCode() and that getMessages().toString() names UnreplayableFailure; the stack-trace message dropped from either catch is red in all three.

question — JEStorage.write has no retry loop. As you read it: one attempt (:913-933), and only a StorageRuntimeException is caught, to unwrap it. Not by design that I can find — the description of #937 records it as the state of things ("JEStorage does not replay at all") when the PDB loop was bounded, and there is no issue for it. JE's LockConflictException is raised once its lock timeout expires and is documented as "abort and retry", so a loop like PDB's belongs there. Filed as #1064 — with one correction to the above: the environment sets je.lock.timeout to 0 (ConfigurableEnvironment.java:359), so a lock wait never times out on JE, and what remains to be replayed is the DeadlockException its deadlock detection raises. Nothing in this change depends on it.

Runs: ReplayedConfigChangeTest 20/20; PDBTestCase, DefaultIndexTest, BulkCursorTest, OnDiskMergeImporterTest, VLVControlTestCase, ServerSideSortControlTestCase — 134 tests, no failures. The reactor run lost PDBTestCase to an admin-port collision with a neighbouring test JVM on this machine (its setUp, Address already in use); run alone afterwards it is 35/35.

[maximthomas]

praise: Every suggestion of the previous round landed, each pinned on the road where it bites.

• aRaisedEntryLimitIsReportedWhenTheWriteWhichAddsIndexesGivesUp (opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:786-816) arms write 1 — "before any of the three writes" is now pinned per write, not on the write-3 road only.
• VLVIndex.getSortKeys() (opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:388) with containsExactly(new SortKey("cn", false)) after the give-up and "sn" after the re-apply (ReplayedConfigChangeTest.java:888, :896): the block moved back to detection time goes red.
• The again twin after the AttributeIndex give-up (ReplayedConfigChangeTest.java:777-783): the raised limit is a change on the second ask, applied, and persistedFlags loses TRUSTED.
• isEqualTo(serverErrorResultCode()) plus contains(UnreplayableFailure.class.getSimpleName()) in both give-up cases: the failure travels next to the rebuild, and the catch cannot answer with any other code.
• CI green on the five ubuntu cells at 97c8773 (run 35238460931).

---

suggestion (non-blocking): The publish-after-the-write placement is pinned for sortKeys only; the baseDN, scope and filter arms are reached by no case.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java:309-320, opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java:1009-1017

Every BackendVLVIndexCfg the test passes comes from vlvIndexCfg(String), which fixes getBaseDN(), getScope() and getFilter() and varies only the sort order; no other test applies a VLV change. So baseDNChanged/scopeChanged/filterChanged are false in every case, and one arm hoisted above the write — say this.filter = newFilter above the try at :290 — survives the suite, while on the give-up road the index would then evaluate searches with a filter neither its tree nor its config holds. The whole-block hoist dies at :888; a one-field hoist does not. My round-2 wording named getSortKeys() as the cheap pin, so this is the remainder of that suggestion, not a new one.

// VLVIndex
SearchFilter getFilter()
{
  return filter;
}

// ReplayedConfigChangeTest — the same give-up road, the change in the filter instead of the sort order
private BackendVLVIndexCfg vlvIndexCfg(String sortOrder, String filter)
{
  final BackendVLVIndexCfg cfg = vlvIndexCfg(sortOrder);
  when(cfg.getFilter()).thenReturn(filter);
  return cfg;
}

backend.storage.failWithoutReplay();
final ConfigChangeResult ccr = vlvIndex.applyConfigurationChange(vlvIndexCfg("+cn", "(cn=*)"));
assertThat(ccr.adminActionRequired()).isTrue();
assertThat(vlvIndex.getFilter().toString()).as("the filter the failed write did not publish")
    .isEqualTo("(objectClass=*)");

Kills the hoisted filter arm; scope and baseDN the same way, one fixture each — or one definition() accessor returning the four and a single isEqualTo per road.

---

suggestion (non-blocking): The confidentiality arm of planIndexUpdates is reached by no case.

opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java:1061-1069

No cfg mock stubs isConfidentialityEnabled(), and the test crypto suite is not encrypted, so setConfidential(false) compares equal in every case and the arm is dead; "arm deleted" is green on the whole class. Origin is the base — the PR re-homed the arm, it did not write it — so a follow-up is fine.

final BackendIndexCfg cfg = indexCfg(newTreeSet(IndexType.EQUALITY), 4000);
when(cfg.isConfidentialityEnabled()).thenReturn(true);   // suite not encrypted: setConfidential(true) is a change
backend.storage.conflictAtCommitOnWrite(3, 1);
final ConfigChangeResult ccr = index.applyConfigurationChange(cfg);

assertThat(ordinalsOf(ccr)).containsOnly(NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD.ordinal());
assertThat(ccr.adminActionRequired()).isTrue();
assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).doesNotContain(TRUSTED);
assertThat(backend.storage.attempts()).isEqualTo(2);

---

question (non-blocking): JEStorage.write has no replay loop (same at the base), so on JE a conflicting untrust write is a first-attempt give-up — the report-before-the-write delivers the instruction there too. Is a JE retry loop a follow-up issue, or does JE stay a first-attempt give-up by design?

[vharseko]

Round 4 pushed as 728dd78, on top of 97c8773. Test-only, plus three package-private getters on VLVIndex.

suggestion — the publish-after-the-write placement is pinned for sortKeys only. Fixed: aChangedSortOrderIsReportedWhenTheWriteWhichUntrustsTheVlvIndexGivesUp (ReplayedConfigChangeTest.java:868-918) now arms a change touching all four published fields at once — a fresh base DN, Scope.SINGLE_LEVEL, "(sn=*)", "+sn" — and reads them back through three new getters, getBaseDN(), getScope(), getFilter() (VLVIndex.java:393-410), next to the existing getSortKeys(). Verified: hoisting this.filter = newFilter above the write — the exact mutant your snippet proposed — turns the give-up assertion red (expected:<"(objectClass=*)"> but was:<"(sn=*)">), and reverting it goes green again. baseDN and scope are guarded by the identical if (xChanged) { this.x = ...; } shape, so the same hoist dies on the corresponding assertion by construction. ReplayedConfigChangeTest 20/20 at head.

suggestion — the confidentiality arm of planIndexUpdates is reached by no case. Repeat of round 3's answer: the fixture cannot reach it as written — EQUALITY plus isConfidentialityEnabled()==true produces equality:hash, an addition and a removal rather than an update — and #1000 replaces the arm's decision with a comparison of the two configurations, adding writes of its own; the pin belongs there, where enablingConfidentialityEmptiesTheIndexItAsksToRebuild already covers it. Nothing changed here.

question — is a JE retry loop a follow-up, or first-attempt give-up by design? Follow-up, already filed. JEStorage.write (opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java:912-933) begins a transaction, runs the operation, commits, and on a StorageRuntimeException rethrows the cause without retrying — exactly issue #1064, with PR #1065 open against it. This PR's report-before-the-write placement already delivers the rebuild instruction correctly on that first-attempt-give-up road, the same way it does on PDB's/JDBC's last-attempt-give-up road; adding the retry loop itself belongs to #1065, not here.

PR description updated to mention the fourth round's pin next to the third's.