[#991] Decide and report an index configuration change outside the write wh...

Valery Kharseko

yesterday b0970cc1a88be6f5dd28edcfdeb635575df575a3

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

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

@@ -14,6 +14,7 @@
 * Copyright 2006-2010 Sun Microsystems, Inc.
 * Portions Copyright 2011-2016 ForgeRock AS.
 * Portions Copyright 2014 Manuel Gaupp
 * Portions Copyright 2026 3A Systems, LLC.
 */
package org.opends.server.backends.pluggable;

@@ -924,18 +925,53 @@
      // indexIdToIndexes
      newIndexIdToIndexes.putAll(updatedIndexes);

      // What the new configuration asks of the indexes which stay is decided here, before any of
      // the three writes below, and reported here as well. Decided before the write which applies
      // it: neither the entry limit an index holds nor its in-memory trusted flag is rolled back
      // with the transaction, while the removal of the persisted TRUSTED flag is, so an attempt
      // which rolls back would leave the raised limit in place, and a replay of it would compare
      // that limit against itself, find nothing to rebuild, and commit an index whose entry limit
      // was raised and which the storage still records as trusted. Reported before the writes
      // rather than once they have committed, because the instruction holds whichever way they go:
      // the configuration entry already holds the raised limit when this listener runs, and the
      // next open of the index applies it to a tree whose keys were given up under the lower one.
      // Only the limit itself waits for the write which untrusts the index to commit.
      final List<Index> indexesToUntrust = new ArrayList<>();
      final List<LocalizableMessage> rebuildMessages = new ArrayList<>();
      planIndexUpdates(updatedIndexes.values(), newConfiguration, indexesToUntrust, rebuildMessages);
      for (LocalizableMessage rebuildMessage : rebuildMessages)
      {
        ccr.setAdminActionRequired(true);
        ccr.addMessage(rebuildMessage);
      }

      // Open added indexes *before* adding them to indexIdToIndexes
      final List<TreeName> addedIndexesToRebuild = new ArrayList<>();
      entryContainer.getRootContainer().getStorage().write(new WriteOperation()
      {
        @Override
        public void run(WriteableTransaction txn) throws Exception
        {
          // Emptied at the start of every attempt: the storage may replay this operation, and what
          // has to be reported is what the attempt which commits found, not what every attempt did.
          addedIndexesToRebuild.clear();
          for (MatchingRuleIndex addedIndex : addedIndexes.values())
          {
            createIndex(txn, addedIndex, ccr);
            if (createIndex(txn, addedIndex))
            {
              addedIndexesToRebuild.add(addedIndex.getName());
            }
          }
        }
      });
      // Reported once that write has committed, since a message an attempt which rolls back added
      // to the result stays there, and the operator would be told once per attempt.
      // EntryContainer.applyConfigurationAdd reports the index it adds the same way.
      for (TreeName addedIndex : addedIndexesToRebuild)
      {
        ccr.setAdminActionRequired(true);
        ccr.addMessage(NOTE_INDEX_ADD_REQUIRES_REBUILD.get(addedIndex));
      }

      config = newConfiguration;
      indexingOptions = newIndexingOptions;
@@ -962,17 +998,28 @@
        entryContainer.unlock();
      }

      entryContainer.getRootContainer().getStorage().write(new WriteOperation()
      // The only part of what the indexes which stay are asked for that is written down. A change
      // which untrusts none of them - a lowered limit - opens no transaction, rather than one a
      // bounded storage could give up on with nothing to give up; VLVIndex guards its write the
      // same way.
      if (!indexesToUntrust.isEmpty())
      {
        @Override
        public void run(WriteableTransaction txn) throws Exception
        entryContainer.getRootContainer().getStorage().write(new WriteOperation()
        {
          for (final Index updatedIndex : updatedIndexes.values())
          @Override
          public void run(WriteableTransaction txn) throws Exception
          {
            updateIndex(updatedIndex, newConfiguration, ccr, txn);
            for (final Index updatedIndex : indexesToUntrust)
            {
              updatedIndex.setTrusted(txn, false);
            }
          }
        }
      });
        });
      }
      for (final Index updatedIndex : updatedIndexes.values())
      {
        updatedIndex.setIndexEntryLimit(newConfiguration.getIndexEntryLimit());
      }
    }
    catch (Exception e)
    {
@@ -983,36 +1030,45 @@
    return ccr;
  }

  private static void createIndex(WriteableTransaction txn, MatchingRuleIndex index, ConfigChangeResult ccr)
  /**
   * Opens an index this change adds, and answers whether it has to be rebuilt before it is used.
   * Answered to the caller rather than reported from here: this runs inside a {@link WriteOperation}
   * the storage may replay, and the report belongs to the attempt which commits.
   */
  private static boolean createIndex(WriteableTransaction txn, MatchingRuleIndex index)
  {
    index.open(txn, true);
    if (!index.isTrusted())
    {
      ccr.setAdminActionRequired(true);
      ccr.addMessage(NOTE_INDEX_ADD_REQUIRES_REBUILD.get(index.getName()));
    }
    return !index.isTrusted();
  }

  private static void updateIndex(Index updatedIndex, BackendIndexCfg newConfig, ConfigChangeResult ccr,
      WriteableTransaction txn)
  /**
   * Works out what the new configuration asks of the indexes which stay: which of them may no longer
   * be trusted, and what the operator has to be told about each of them. Decided from the state the
   * indexes are in before anything is applied to them, so that a write the storage replays reaches
   * the same answer on every attempt.
   */
  private static void planIndexUpdates(Collection<MatchingRuleIndex> updatedIndexes, BackendIndexCfg newConfig,
      List<Index> indexesToUntrust, List<LocalizableMessage> rebuildMessages)
  {
    // This index could still be used since a new smaller index size limit doesn't impact validity of the results.
    boolean newLimitRequiresRebuild = updatedIndex.setIndexEntryLimit(newConfig.getIndexEntryLimit());
    if (newLimitRequiresRebuild)
    for (Index updatedIndex : updatedIndexes)
    {
      ccr.setAdminActionRequired(true);
      ccr.addMessage(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.get(updatedIndex.getName()));
    }
    // This index could still be used when disabling confidentiality.
    boolean newConfidentialityRequiresRebuild = updatedIndex.setConfidential(newConfig.isConfidentialityEnabled());
    if (newConfidentialityRequiresRebuild)
    {
      ccr.setAdminActionRequired(true);
      ccr.addMessage(NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD.get(updatedIndex.getName()));
    }
    if (newLimitRequiresRebuild || newConfidentialityRequiresRebuild)
    {
      updatedIndex.setTrusted(txn, false);
      // This index could still be used since a new smaller index size limit doesn't impact validity of the results.
      boolean newLimitRequiresRebuild = updatedIndex.getIndexEntryLimit() < newConfig.getIndexEntryLimit();
      if (newLimitRequiresRebuild)
      {
        rebuildMessages.add(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.get(updatedIndex.getName()));
      }
      // This index could still be used when disabling confidentiality. Asked rather than told: for an
      // index this only compares the configuration with the parameters its crypto suite holds.
      boolean newConfidentialityRequiresRebuild = updatedIndex.setConfidential(newConfig.isConfidentialityEnabled());
      if (newConfidentialityRequiresRebuild)
      {
        rebuildMessages.add(NOTE_CONFIG_INDEX_CONFIDENTIALITY_REQUIRES_REBUILD.get(updatedIndex.getName()));
      }
      if (newLimitRequiresRebuild || newConfidentialityRequiresRebuild)
      {
        indexesToUntrust.add(updatedIndex);
      }
    }
  }


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

@@ -65,7 +65,6 @@
import org.opends.server.controls.ServerSideSortRequestControl;
import org.opends.server.controls.VLVRequestControl;
import org.opends.server.controls.VLVResponseControl;
import org.opends.server.core.DirectoryServer;
import org.opends.server.core.SearchOperation;
import org.opends.server.protocols.ldap.LDAPResultCode;
import org.opends.server.types.Attribute;
@@ -73,7 +72,6 @@
import org.opends.server.types.Entry;
import org.opends.server.types.Modification;
import org.opends.server.types.SearchFilter;
import org.opends.server.util.StaticUtils;

/**
 * This class represents a VLV index.
@@ -229,71 +227,103 @@
  @Override
  public synchronized ConfigChangeResult applyConfigurationChange(final BackendVLVIndexCfg cfg)
  {
    try
    final ConfigChangeResult ccr = new ConfigChangeResult();
    /*
     * What this change asks for is worked out here, before the write which applies it, and what it
     * changes is published after that write has committed. Asked and answered from within a
     * WriteOperation the storage may replay, every question below is asked of the configuration
     * this vlvIndex holds and answered into the result of the change, and neither is rolled back
     * with the transaction: an attempt which rolls back leaves this vlvIndex already holding the
     * new definition, so the replay of it finds nothing changed, and it leaves the result already
     * asking for the rebuild, which is the only thing that keeps the replay removing the TRUSTED
     * flag the rollback put back. The operator is told to rebuild the index once per attempt, and
     * what stops the storage from committing a vlvIndex it still records as trusted - answering a
     * sorted search after a restart out of a tree built for the definition it no longer has - is
     * that repetition. See OpenDJ issue #991, which reports this of AttributeIndex, where the
     * index itself holds the answer and the replay does commit a stale index as trusted.
     */
    final boolean baseDNChanged = !config.getBaseDN().equals(cfg.getBaseDN());
    if (baseDNChanged)
    {
      final ConfigChangeResult ccr = new ConfigChangeResult();
      storage.write(new WriteOperation()
      {
        @Override
        public void run(final WriteableTransaction txn) throws Exception
        {
          applyConfigurationChange0(txn, cfg, ccr);
        }
      });
      return ccr;
    }
    catch (final Exception e)
    {
      throw new StorageRuntimeException(e);
    }
  }

  private synchronized void applyConfigurationChange0(
      final WriteableTransaction txn, final BackendVLVIndexCfg cfg, final ConfigChangeResult ccr)
  {
    // Update base DN only if changed
    if (!config.getBaseDN().equals(cfg.getBaseDN()))
    {
      this.baseDN = cfg.getBaseDN();
      ccr.setAdminActionRequired(true);
    }

    // Update scope only if changed
    if (!config.getScope().equals(cfg.getScope()))
    final boolean scopeChanged = !config.getScope().equals(cfg.getScope());
    if (scopeChanged)
    {
      this.scope = convertScope(cfg.getScope());
      ccr.setAdminActionRequired(true);
    }

    // Update the filter only if changed
    if (!config.getFilter().equals(cfg.getFilter()))
    // parseSearchFilter() asks for the administrative action itself, and only once it has parsed.
    final boolean filterChanged = !config.getFilter().equals(cfg.getFilter());
    final SearchFilter newFilter = filterChanged ? parseSearchFilter(cfg, getName().toString(), ccr) : filter;
    final boolean sortOrderChanged = !config.getSortOrder().equals(cfg.getSortOrder());
    final List<SortKey> newSortKeys;
    if (sortOrderChanged)
    {
      this.filter = parseSearchFilter(cfg, getName().toString(), ccr);
    }

    // Update the sort order only if changed
    if (!config.getSortOrder().equals(cfg.getSortOrder()))
    {
      this.sortKeys = parseSortKeys(cfg.getSortOrder(), ccr);
      newSortKeys = parseSortKeys(cfg.getSortOrder(), ccr);
      ccr.setAdminActionRequired(true);
    }

    if (ccr.adminActionRequired())
    else
    {
      trusted = false;
      newSortKeys = sortKeys;
    }

    final boolean requiresRebuild = ccr.adminActionRequired();
    if (requiresRebuild)
    {
      // Reported outside the write rather than from within it, since a message an attempt which
      // rolls back added to the result stays there and the operator would be told once per
      // attempt; and before the write rather than once it has committed, because the instruction
      // holds whichever way the write goes: the configuration entry already holds the new
      // definition when this listener runs, and the next open of this vlvIndex applies it to a
      // tree built for the definition it no longer has.
      ccr.addMessage(NOTE_INDEX_ADD_REQUIRES_REBUILD.get(getName()));

      // The only part of this change which is written down. A change asking for nothing this
      // vlvIndex has to be rebuilt for opens no transaction, rather than one a bounded storage
      // could give up on with nothing to give up. A conflict raised by the flag removal is left to
      // the storage, whose retry loop replays the operation: caught inside the operation, as it
      // used to be, it was swallowed where the storage was waiting to be told to replay, and the
      // attempt committed having done nothing. What the storage gives up on is reported the way
      // AttributeIndex reports it, with the result built so far - the rebuild asked for above
      // holds on that road too - rather than thrown past ConfigurationHandler, which catches
      // nothing a listener throws and would discard that result whole.
      try
      {
        state.removeFlagsFromIndex(txn, getName(), IndexFlag.TRUSTED);
        storage.write(new WriteOperation()
        {
          @Override
          public void run(final WriteableTransaction txn) throws Exception
          {
            setTrusted(txn, false);
          }
        });
      }
      catch (final StorageRuntimeException de)
      catch (final Exception e)
      {
        ccr.addMessage(LocalizableMessage.raw(StaticUtils.stackTraceToSingleLineString(de)));
        ccr.setResultCodeIfSuccess(DirectoryServer.getCoreConfigManager().getServerErrorResultCode());
        ccr.setResultCode(getCoreConfigManager().getServerErrorResultCode());
        ccr.addMessage(LocalizableMessage.raw(stackTraceToSingleLineString(e)));
        return ccr;
      }
    }

    if (baseDNChanged)
    {
      this.baseDN = cfg.getBaseDN();
    }
    if (scopeChanged)
    {
      this.scope = convertScope(cfg.getScope());
    }
    if (filterChanged)
    {
      this.filter = newFilter;
    }
    if (sortOrderChanged)
    {
      this.sortKeys = newSortKeys;
    }
    this.config = cfg;
    return ccr;
  }

  private List<SortKey> parseSortKeys(final String sortOrder, ConfigChangeResult ccr)
@@ -354,6 +384,30 @@
    return trusted;
  }

  /** The sort keys this vlvIndex encodes its keys with: the definition the last committed change published. */
  List<SortKey> getSortKeys()
  {
    return sortKeys;
  }

  /** The base DN this vlvIndex searches under: the definition the last committed change published. */
  DN getBaseDN()
  {
    return baseDN;
  }

  /** The scope this vlvIndex searches with: the definition the last committed change published. */
  SearchScope getScope()
  {
    return scope;
  }

  /** The filter this vlvIndex searches with: the definition the last committed change published. */
  SearchFilter getFilter()
  {
    return filter;
  }

  synchronized void setTrusted(final WriteableTransaction txn, final boolean trusted) throws StorageRuntimeException
  {
    this.trusted = trusted;

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

@@ -19,6 +19,8 @@
import static org.opends.messages.BackendMessages.ERR_BACKEND_BASEDN_NO_LONGER_HELD;
import static org.opends.messages.BackendMessages.ERR_BACKEND_CANNOT_LIST_TREES_AFTER_BASEDN_CHANGE;
import static org.opends.messages.BackendMessages.ERR_BACKEND_CANNOT_REGISTER_BASEDN;
import static org.opends.messages.BackendMessages.NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD;
import static org.opends.messages.BackendMessages.NOTE_INDEX_ADD_REQUIRES_REBUILD;
import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
@@ -44,13 +46,18 @@
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.DN;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SearchScope;
import org.forgerock.opendj.ldap.SortKey;
import org.forgerock.opendj.ldap.schema.AttributeType;
import org.forgerock.opendj.server.config.meta.BackendIndexCfgDefn.IndexType;
import org.forgerock.opendj.server.config.meta.BackendVLVIndexCfgDefn.Scope;
import org.forgerock.opendj.server.config.server.BackendIndexCfg;
import org.forgerock.opendj.server.config.server.BackendVLVIndexCfg;
import org.forgerock.opendj.server.config.server.PDBBackendCfg;
import org.opends.server.DirectoryServerTestCase;
import org.opends.server.TestCaseUtils;
import org.opends.server.backends.pdb.PDBStorage;
import org.opends.server.backends.pluggable.AttributeIndex.MatchingRuleIndex;
import org.opends.server.backends.pluggable.State.IndexFlag;
import org.opends.server.backends.pluggable.spi.AccessMode;
import org.opends.server.backends.pluggable.spi.Cursor;
@@ -63,6 +70,8 @@
import org.opends.server.backends.pluggable.spi.UpdateFunction;
import org.opends.server.backends.pluggable.spi.WriteOperation;
import org.opends.server.backends.pluggable.spi.WriteableTransaction;
import org.opends.server.core.AddOperation;
import org.opends.server.core.DirectoryServer;
import org.opends.server.core.ServerContext;
import org.opends.server.types.BackupConfig;
import org.opends.server.types.BackupDirectory;
@@ -95,6 +104,8 @@
  private static final DN ADDED = DN.valueOf("dc=b907c,dc=com");
  /** Hierarchically related to {@link #KEPT}, which one backend is not allowed to serve as well. */
  private static final DN UNREGISTRABLE = DN.valueOf("dc=b907d,dc=b907a,dc=com");
  /** Held in lower case, which is how an entry container keys the vlvIndexes it holds. */
  private static final String VLV_INDEX_NAME = "b907vlv";

  private ServerContext serverContext;
  private AttributeType cnType;
@@ -545,6 +556,16 @@
    }
  }

  private static Set<TreeName> treesOf(EntryContainer ec)
  {
    final Set<TreeName> names = new HashSet<>();
    for (Tree tree : ec.listTrees())
    {
      names.add(tree.getName());
    }
    return names;
  }

  /** The messages a change result carries, by identity rather than by their formatted text. */
  private static Set<Integer> ordinalsOf(ConfigChangeResult ccr)
  {
@@ -556,14 +577,344 @@
    return ordinals;
  }

  private static Set<TreeName> treesOf(EntryContainer ec)
  /** The result code a configuration listener answers a failure it caught with. */
  private static ResultCode serverErrorResultCode()
  {
    final Set<TreeName> names = new HashSet<>();
    for (Tree tree : ec.listTrees())
    return DirectoryServer.getCoreConfigManager().getServerErrorResultCode();
  }

  /**
   * An index a change adds is opened untrusted while the backend holds entries, and the operator is
   * told to rebuild it. That message belongs to the attempt which commits: added to the result from
   * inside the operation, a replay repeats it once per attempt.
   */
  @Test
  public void anIndexAddedByAChangeIsReportedOnceWhenTheTransactionIsReplayed() throws Exception
  {
    final ReplayingBackend backend = openBackend(newTreeSet(KEPT));
    try
    {
      names.add(tree.getName());
      addBaseEntry(backend, KEPT, "b907a");
      final AttributeIndex index = backend.getRootContainer().getEntryContainer(KEPT).getAttributeIndex(cnType);

      // The first of the three writes a change makes is the one which opens the indexes it adds.
      backend.storage.conflictAtCommit(1);
      final ConfigChangeResult ccr =
          index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.PRESENCE), 4000));

      assertThat(backend.storage.attempts()).isEqualTo(2);
      assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(ccr.adminActionRequired()).isTrue();
      assertThat(ccr.getMessages()).as("the rebuild the added index needs, asked for once").hasSize(1);
      assertThat(ordinalsOf(ccr)).containsOnly(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal());
      assertThat(index.isIndexed(IndexType.PRESENCE)).as("the index type the change added").isTrue();
    }
    return names;
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * Raising the entry limit of an index leaves it holding the keys it gave up under the lower limit,
   * so it has to be rebuilt and may not be trusted until it is. The limit an index holds is not
   * rolled back with the transaction while the removal of its persisted TRUSTED flag is, so an
   * attempt which rolls back must not be what decides that the limit went up.
   */
  @Test
  public void aRaisedEntryLimitUntrustsTheIndexWhenTheTransactionIsReplayed() throws Exception
  {
    final ReplayingBackend backend = openBackend(newTreeSet(KEPT));
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final AttributeIndex index = ec.getAttributeIndex(cnType);
      final MatchingRuleIndex cnIndex = index.getNameToIndexes().values().iterator().next();
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).contains(TRUSTED);

      // The third write is the one which removes the flag: the first two open the indexes the
      // change adds and delete the ones it removes, and it neither adds nor removes any.
      final int writesBefore = backend.storage.writes();
      backend.storage.conflictAtCommitOnWrite(3, 1);
      final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));

      assertThat(backend.storage.writes()).as("the armed write was the last of three").isEqualTo(writesBefore + 3);
      assertThat(backend.storage.attempts()).isEqualTo(2);
      assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(ccr.adminActionRequired()).isTrue();
      assertThat(ccr.getMessages()).as("the rebuild the raised limit needs, asked for once").hasSize(1);
      assertThat(ordinalsOf(ccr)).containsOnly(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
      assertThat(cnIndex.getIndexEntryLimit()).as("the limit the change applied").isEqualTo(8000);
      assertThat(cnIndex.isTrusted()).isFalse();
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName()))
          .as("the flag the attempt which committed had to remove").doesNotContain(TRUSTED);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * A vlvIndex whose sort order changes holds a tree sorted under the order it no longer has, so it
   * may not be trusted until it is rebuilt. Every question it asks is asked of the configuration it
   * holds, and no rollback takes that configuration back, so the replay of an attempt which rolled
   * back finds nothing changed: the rebuild it goes on asking for is the administrative action the
   * rolled back attempt left in the result, told to the operator once per attempt.
   */
  @Test
  public void aChangedSortOrderUntrustsTheVlvIndexWhenTheTransactionIsReplayed() throws Exception
  {
    final ReplayingBackend backend = openBackendWithVlvIndex();
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final VLVIndex vlvIndex = ec.getVLVIndex(VLV_INDEX_NAME);
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName())).contains(TRUSTED);

      backend.storage.conflictAtCommit(1);
      final ConfigChangeResult ccr = vlvIndex.applyConfigurationChange(vlvIndexCfg("+sn"));

      assertThat(backend.storage.attempts()).isEqualTo(2);
      assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(ccr.adminActionRequired()).isTrue();
      assertThat(ccr.getMessages()).as("the rebuild the new sort order needs, asked for once").hasSize(1);
      assertThat(ordinalsOf(ccr)).containsOnly(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal());
      assertThat(vlvIndex.isTrusted()).isFalse();
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName()))
          .as("the flag the attempt which committed had to remove").doesNotContain(TRUSTED);
      assertThat(vlvIndex.getSortKeys()).as("the definition the committed write published")
          .containsExactly(new SortKey("sn", false));

      // The definition the change applied is the one this vlvIndex holds from now on, so asking for
      // it a second time asks for nothing, and opens no transaction to commit nothing.
      final int writesBefore = backend.storage.writes();
      final ConfigChangeResult again = vlvIndex.applyConfigurationChange(vlvIndexCfg("+sn"));
      assertThat(again.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(again.adminActionRequired()).as("a change which changes nothing").isFalse();
      assertThat(again.getMessages()).isEmpty();
      assertThat(backend.storage.writes()).as("a change which changes nothing opens no transaction")
          .isEqualTo(writesBefore);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * A conflict the flag removal itself raises is reported to the operation by the transaction, as
   * a {@link StorageRuntimeException} wrapping it, and belongs to the storage's retry loop. Caught
   * inside the operation and turned into a message, it was a conflict swallowed where the storage
   * was waiting to be told to replay: the attempt committed having removed nothing, and the change
   * reported a failure against a vlvIndex the storage still recorded as trusted.
   */
  @Test
  public void aConflictRaisedByTheRemovalOfTheVlvIndexFlagIsReplayed() throws Exception
  {
    final ReplayingBackend backend = openBackendWithVlvIndex();
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final VLVIndex vlvIndex = ec.getVLVIndex(VLV_INDEX_NAME);
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName())).contains(TRUSTED);

      backend.storage.conflictAsTheTransactionReportsIt(1);
      final ConfigChangeResult ccr = vlvIndex.applyConfigurationChange(vlvIndexCfg("+sn"));

      assertThat(backend.storage.attempts()).as("replayed by the storage, not answered from inside").isEqualTo(2);
      assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(ccr.adminActionRequired()).isTrue();
      assertThat(ccr.getMessages()).as("the rebuild the new sort order needs, asked for once").hasSize(1);
      assertThat(ordinalsOf(ccr)).containsOnly(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal());
      assertThat(vlvIndex.isTrusted()).isFalse();
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName()))
          .as("the flag the replayed attempt removed").doesNotContain(TRUSTED);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * A raised entry limit has to be rebuilt for whether or not the write which untrusts the index is
   * applied: the configuration entry holds the raised limit before the listener runs, and the next
   * open of the index applies it to a tree whose keys were given up under the lower one. So the
   * instruction is given before that write, and a write the storage gives up on - the last attempt
   * of its retry loop, or a failure it does not replay at all - leaves it in the result, next to
   * the failure, rather than dropping it together with the limit it could not apply.
   */
  @Test
  public void aRaisedEntryLimitIsReportedWhenTheWriteWhichUntrustsTheIndexGivesUp() throws Exception
  {
    final ReplayingBackend backend = openBackend(newTreeSet(KEPT));
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final AttributeIndex index = ec.getAttributeIndex(cnType);
      final MatchingRuleIndex cnIndex = index.getNameToIndexes().values().iterator().next();
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).contains(TRUSTED);

      backend.storage.failWithoutReplayOnWrite(3);
      final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));

      assertThat(ccr.getResultCode()).isEqualTo(serverErrorResultCode());
      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(ccr.getMessages().toString()).as("the failure, next to the rebuild")
          .contains(UnreplayableFailure.class.getSimpleName());
      assertThat(cnIndex.getIndexEntryLimit()).as("the limit the failed write did not apply").isEqualTo(4000);
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName()))
          .as("what the restart will read").contains(TRUSTED);

      // The limit the failed write did not apply is still a change when asked for again: the
      // configuration this attribute index holds was replaced before the write which gave up, and
      // what the new one asks of the index is asked of the index, not of that configuration.
      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)).containsOnly(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
      assertThat(cnIndex.getIndexEntryLimit()).as("the limit the change applied").isEqualTo(8000);
      assertThat(cnIndex.isTrusted()).isFalse();
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).doesNotContain(TRUSTED);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * The same instruction survives a give-up on the first of the three writes, the one which opens
   * the indexes the change adds - opened here with none to add: it is asked for before any of the
   * writes, not only before the one which untrusts the index, since the configuration entry holds
   * the raised limit whichever of them fails.
   */
  @Test
  public void aRaisedEntryLimitIsReportedWhenTheWriteWhichAddsIndexesGivesUp() throws Exception
  {
    final ReplayingBackend backend = openBackend(newTreeSet(KEPT));
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final AttributeIndex index = ec.getAttributeIndex(cnType);
      final MatchingRuleIndex cnIndex = index.getNameToIndexes().values().iterator().next();
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName())).contains(TRUSTED);

      final int writesBefore = backend.storage.writes();
      backend.storage.failWithoutReplayOnWrite(1);
      final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));

      assertThat(backend.storage.writes()).as("the armed write was the first of three, and the last one made")
          .isEqualTo(writesBefore + 1);
      assertThat(ccr.getResultCode()).isEqualTo(serverErrorResultCode());
      assertThat(ccr.adminActionRequired()).as("the rebuild the raised limit needs, asked for before the first write")
          .isTrue();
      assertThat(ordinalsOf(ccr)).contains(NOTE_CONFIG_INDEX_ENTRY_LIMIT_REQUIRES_REBUILD.ordinal());
      assertThat(ccr.getMessages().toString()).as("the failure, next to the rebuild")
          .contains(UnreplayableFailure.class.getSimpleName());
      assertThat(cnIndex.getIndexEntryLimit()).as("the limit the failed change did not apply").isEqualTo(4000);
      assertThat(persistedFlags(rootContainer, ec, cnIndex.getName()))
          .as("what the restart will read").contains(TRUSTED);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * A lowered entry limit leaves every key the index holds valid, so it asks for no rebuild, and it
   * untrusts nothing, so it opens no transaction: nothing for a bounded storage to give up on.
   */
  @Test
  public void aLoweredEntryLimitNeedsNoRebuildAndOpensNoTransaction() throws Exception
  {
    final ReplayingBackend backend = openBackend(newTreeSet(KEPT));
    try
    {
      final AttributeIndex index = backend.getRootContainer().getEntryContainer(KEPT).getAttributeIndex(cnType);
      final MatchingRuleIndex cnIndex = index.getNameToIndexes().values().iterator().next();

      final int writesBefore = backend.storage.writes();
      final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 2000));

      assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(ccr.adminActionRequired()).as("a lowered limit needs no rebuild").isFalse();
      assertThat(ccr.getMessages()).isEmpty();
      assertThat(cnIndex.isTrusted()).isTrue();
      assertThat(cnIndex.getIndexEntryLimit()).as("the limit the change applied").isEqualTo(2000);
      assertThat(backend.storage.writes()).as("the two writes which add and remove indexes, and no third")
          .isEqualTo(writesBefore + 2);
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /**
   * The same road for a vlvIndex: the write which untrusts it is the only one the change makes, and
   * a failure of it is reported with the rebuild the change asked for, rather than thrown out of
   * the listener with that result discarded. The change touches all four published fields at once,
   * so none of them can be hoisted above the write and still leave this case green.
   */
  @Test
  public void aChangedSortOrderIsReportedWhenTheWriteWhichUntrustsTheVlvIndexGivesUp() throws Exception
  {
    final ReplayingBackend backend = openBackendWithVlvIndex();
    try
    {
      final RootContainer rootContainer = backend.getRootContainer();
      final EntryContainer ec = rootContainer.getEntryContainer(KEPT);
      final VLVIndex vlvIndex = ec.getVLVIndex(VLV_INDEX_NAME);
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName())).contains(TRUSTED);

      // A base DN of the vlvIndex's own change, unrelated to REMOVED's meaning elsewhere in this suite.
      final DN newBaseDN = DN.valueOf("dc=b907e,dc=com");
      final BackendVLVIndexCfg changedCfg = vlvIndexCfg("+sn", newBaseDN, Scope.SINGLE_LEVEL, "(sn=*)");
      backend.storage.failWithoutReplay();
      final ConfigChangeResult ccr = vlvIndex.applyConfigurationChange(changedCfg);

      assertThat(ccr.getResultCode()).isEqualTo(serverErrorResultCode());
      assertThat(ccr.adminActionRequired())
          .as("the rebuild the new sort order needs, on the road which failed").isTrue();
      assertThat(ordinalsOf(ccr)).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal());
      assertThat(ccr.getMessages().toString()).as("the failure, next to the rebuild")
          .contains(UnreplayableFailure.class.getSimpleName());
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName()))
          .as("what the restart will read").contains(TRUSTED);
      assertThat((Object) vlvIndex.getBaseDN()).as("the base DN the failed write did not publish").isEqualTo(KEPT);
      assertThat(vlvIndex.getScope()).as("the scope the failed write did not publish")
          .isEqualTo(SearchScope.WHOLE_SUBTREE);
      assertThat(vlvIndex.getFilter().toString()).as("the filter the failed write did not publish")
          .isEqualTo("(objectClass=*)");
      assertThat(vlvIndex.getSortKeys()).as("the definition the failed write did not publish")
          .containsExactly(new SortKey("cn", false));

      // The definition the failed change did not publish is still a change when asked for again.
      final ConfigChangeResult again = vlvIndex.applyConfigurationChange(changedCfg);
      assertThat(again.getResultCode()).isEqualTo(ResultCode.SUCCESS);
      assertThat(again.adminActionRequired()).as("the definition the failed write did not publish").isTrue();
      assertThat(persistedFlags(rootContainer, ec, vlvIndex.getName())).doesNotContain(TRUSTED);
      assertThat((Object) vlvIndex.getBaseDN()).as("the base DN the write which committed published").isEqualTo(newBaseDN);
      assertThat(vlvIndex.getScope()).as("the scope the write which committed published")
          .isEqualTo(SearchScope.SINGLE_LEVEL);
      assertThat(vlvIndex.getFilter().toString()).as("the filter the write which committed published")
          .isEqualTo("(sn=*)");
      assertThat(vlvIndex.getSortKeys()).as("the definition the write which committed published")
          .containsExactly(new SortKey("sn", false));
    }
    finally
    {
      backend.finalizeBackend();
    }
  }

  /** Reads back the flags an index was given when it was opened, as they are stored. */
@@ -576,9 +927,20 @@

  private ReplayingBackend openBackend(SortedSet<DN> baseDNs) throws Exception
  {
    return openBackend(baseDNs, false);
  }

  /** The vlvIndex is opened only where a test is about one, since every base DN gets a copy of it. */
  private ReplayingBackend openBackendWithVlvIndex() throws Exception
  {
    return openBackend(newTreeSet(KEPT), true);
  }

  private ReplayingBackend openBackend(SortedSet<DN> baseDNs, boolean withVlvIndex) throws Exception
  {
    final ReplayingBackend backend = new ReplayingBackend();
    backend.setBackendID(BACKEND_ID);
    backend.configuredWith = backendCfg(baseDNs);
    backend.configuredWith = backendCfg(baseDNs, withVlvIndex);
    backend.configureBackend(backend.configuredWith, serverContext);
    // Start from a pristine on-disk state so that a previous run cannot mask the defect.
    backend.storage.removeStorageFiles();
@@ -618,6 +980,11 @@

  private PDBBackendCfg backendCfg(SortedSet<DN> baseDNs) throws ConfigException
  {
    return backendCfg(baseDNs, false);
  }

  private PDBBackendCfg backendCfg(SortedSet<DN> baseDNs, boolean withVlvIndex) throws ConfigException
  {
    final PDBBackendCfg cfg = mockCfg(PDBBackendCfg.class);
    when(cfg.dn()).thenReturn(DN.valueOf("ds-cfg-backend-id=" + BACKEND_ID + ",cn=Backends,cn=config"));
    when(cfg.getBackendId()).thenReturn(BACKEND_ID);
@@ -627,17 +994,58 @@
    when(cfg.getDBCachePercent()).thenReturn(20);
    when(cfg.getBaseDN()).thenReturn(baseDNs);
    when(cfg.listBackendIndexes()).thenReturn(new String[] { "cn" });
    when(cfg.listBackendVLVIndexes()).thenReturn(new String[0]);

    final BackendIndexCfg indexCfg = mock(BackendIndexCfg.class);
    when(indexCfg.getIndexType()).thenReturn(newTreeSet(IndexType.EQUALITY));
    when(indexCfg.getAttribute()).thenReturn(cnType);
    when(indexCfg.getIndexEntryLimit()).thenReturn(4000);
    when(indexCfg.getSubstringLength()).thenReturn(6);
    when(cfg.getBackendIndex("cn")).thenReturn(indexCfg);
    // Built before it is handed over: stubbing a mock from inside a when() of another mock leaves
    // that when() unfinished, and Mockito fails the next test to touch either of them.
    final BackendIndexCfg cnIndexCfg = indexCfg(newTreeSet(IndexType.EQUALITY), 4000);
    when(cfg.getBackendIndex("cn")).thenReturn(cnIndexCfg);
    if (withVlvIndex)
    {
      final BackendVLVIndexCfg vlvCfg = vlvIndexCfg("+cn");
      when(cfg.listBackendVLVIndexes()).thenReturn(new String[] { VLV_INDEX_NAME });
      when(cfg.getBackendVLVIndex(VLV_INDEX_NAME)).thenReturn(vlvCfg);
    }
    else
    {
      when(cfg.listBackendVLVIndexes()).thenReturn(new String[0]);
    }
    return cfg;
  }

  private BackendIndexCfg indexCfg(SortedSet<IndexType> indexTypes, int indexEntryLimit)
  {
    final BackendIndexCfg cfg = mock(BackendIndexCfg.class);
    when(cfg.getIndexType()).thenReturn(indexTypes);
    when(cfg.getAttribute()).thenReturn(cnType);
    when(cfg.getIndexEntryLimit()).thenReturn(indexEntryLimit);
    when(cfg.getSubstringLength()).thenReturn(6);
    return cfg;
  }

  private BackendVLVIndexCfg vlvIndexCfg(String sortOrder)
  {
    return vlvIndexCfg(sortOrder, KEPT, Scope.WHOLE_SUBTREE, "(objectClass=*)");
  }

  /** Varies the three fields a plain {@link #vlvIndexCfg(String)} change leaves alone, next to the sort order. */
  private BackendVLVIndexCfg vlvIndexCfg(String sortOrder, DN baseDN, Scope scope, String filter)
  {
    final BackendVLVIndexCfg cfg = mock(BackendVLVIndexCfg.class);
    when(cfg.getName()).thenReturn(VLV_INDEX_NAME);
    when(cfg.getBaseDN()).thenReturn(baseDN);
    when(cfg.getScope()).thenReturn(scope);
    when(cfg.getFilter()).thenReturn(filter);
    when(cfg.getSortOrder()).thenReturn(sortOrder);
    return cfg;
  }

  /** An index of an empty backend is trusted when it is opened, whatever its tree holds. */
  private static void addBaseEntry(ReplayingBackend backend, DN baseDN, String domainComponent) throws Exception
  {
    backend.addEntry(
        TestCaseUtils.makeEntry("dn: " + baseDN, "objectClass: top", "objectClass: domain", "dc: " + domainComponent),
        mock(AddOperation.class));
  }

  /** A backend whose storage makes the next write operation conflict, and so be replayed. */
  private static final class ReplayingBackend extends BackendImpl<PDBBackendCfg>
  {
@@ -709,6 +1117,13 @@
    {
      /** As soon as the operation first touches the transaction, before it has changed anything. */
      FIRST_STORAGE_ACCESS,
      /**
       * As soon as the operation first touches the transaction, in the form the transaction of
       * {@code PDBStorage} reports a conflict: a {@link StorageRuntimeException} wrapping the
       * {@link RollbackException}. What an operation catching the former from inside sees, where
       * {@link #FIRST_STORAGE_ACCESS} raises the bare conflict such a catch never meets.
       */
      FIRST_STORAGE_ACCESS_AS_THE_TRANSACTION_REPORTS_IT,
      /** Once the operation has run to completion, as a conflict reported by {@code commit()}. */
      COMMIT,
      /** Once the operation has run to completion, as a failure which is not replayed at all. */
@@ -723,6 +1138,8 @@
    private final Storage delegate;
    private Runnable onListTrees;
    private ConflictPoint conflictPoint;
    /** Which write, counted over the life of this storage, is armed; zero for the next one. */
    private int armedWrite;
    private int conflictsLeft;
    private int attempts;
    private int writes;
@@ -737,16 +1154,46 @@
      arm(ConflictPoint.FIRST_STORAGE_ACCESS, conflicts);
    }

    /**
     * Conflicts at the first storage access of the operation, in the form the transaction itself
     * reports one: what the operation sees where it asks the transaction for something, rather than
     * what the retry loop sees once the operation has let it through.
     */
    void conflictAsTheTransactionReportsIt(int conflicts)
    {
      arm(ConflictPoint.FIRST_STORAGE_ACCESS_AS_THE_TRANSACTION_REPORTS_IT, conflicts);
    }

    void conflictAtCommit(int conflicts)
    {
      arm(ConflictPoint.COMMIT, conflicts);
    }

    /**
     * Conflicts at commit time on the {@code nth} write asked for from now on, the next one being
     * the first: a configuration change which makes several writes has to arm the one it is about.
     */
    void conflictAtCommitOnWrite(int nth, int conflicts)
    {
      arm(ConflictPoint.COMMIT, conflicts);
      armedWrite = writes + nth;
    }

    void failWithoutReplay()
    {
      arm(ConflictPoint.NO_REPLAY, 1);
    }

    /**
     * Fails without a replay on the {@code nth} write asked for from now on, the next one being
     * the first: what the last attempt of a retry loop which gave up leaves the caller holding.
     */
    void failWithoutReplayOnWrite(int nth)
    {
      arm(ConflictPoint.NO_REPLAY, 1);
      armedWrite = writes + nth;
    }

    void failAfterCommit()
    {
      arm(ConflictPoint.NO_REPLAY_AFTER_COMMIT, 1);
@@ -756,6 +1203,7 @@
    {
      conflictPoint = where;
      conflictsLeft = conflicts;
      armedWrite = 0;
      attempts = 0;
    }

@@ -776,12 +1224,13 @@
    {
      writes++;
      final ConflictPoint armed = conflictPoint;
      if (armed == null)
      if (armed == null || (armedWrite != 0 && writes != armedWrite))
      {
        delegate.write(writeOperation);
        return;
      }
      conflictPoint = null;
      armedWrite = 0;
      if (armed == ConflictPoint.NO_REPLAY_AFTER_COMMIT)
      {
        // Committed, then reported as a failure: the operation's work outlives the failure, as it
@@ -812,7 +1261,12 @@
          }
          if (armed == ConflictPoint.FIRST_STORAGE_ACCESS)
          {
            writeOperation.run(new ConflictingTransaction());
            writeOperation.run(new ConflictingTransaction(false));
            return;
          }
          if (armed == ConflictPoint.FIRST_STORAGE_ACCESS_AS_THE_TRANSACTION_REPORTS_IT)
          {
            writeOperation.run(new ConflictingTransaction(true));
            return;
          }
          writeOperation.run(txn);
@@ -905,9 +1359,21 @@
  /** A transaction which conflicts as soon as it is used, without ever reaching the storage. */
  private static final class ConflictingTransaction implements WriteableTransaction
  {
    private static RollbackException conflict()
    /**
     * Whether the conflict is raised as {@code PDBStorage}'s transaction raises it - wrapped in a
     * {@link StorageRuntimeException}, which that storage's retry loop unwraps - or bare.
     */
    private final boolean asTheTransactionReportsIt;

    ConflictingTransaction(boolean asTheTransactionReportsIt)
    {
      return new RollbackException();
      this.asTheTransactionReportsIt = asTheTransactionReportsIt;
    }

    private RuntimeException conflict()
    {
      final RollbackException conflict = new RollbackException();
      return asTheTransactionReportsIt ? new StorageRuntimeException(conflict) : conflict;
    }

    @Override