Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vharseko wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
026d538
[#991] Decide and report an index configuration change outside the wr…
vharseko Sep 9, 2026
b7094cd
[#991] Report the rebuild before the writes, and report a give-up on …
vharseko Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 88 additions & 32 deletions opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -924,18 +925,53 @@ public synchronized ConfigChangeResult applyConfigurationChange(final BackendInd
// 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;
Expand All @@ -962,17 +998,28 @@ public void run(WriteableTransaction txn) throws Exception
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)
{
Expand All @@ -983,36 +1030,45 @@ public void run(WriteableTransaction txn) throws Exception
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)
{
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)
for (Index updatedIndex : updatedIndexes)
{
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);
}
}
}

Expand Down
122 changes: 76 additions & 46 deletions opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@
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;
import org.opends.server.types.DirectoryException;
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.
Expand Down Expand Up @@ -229,71 +227,103 @@ private static SearchFilter parseSearchFilter(final BackendVLVIndexCfg cfg, Stri
@Override
public synchronized ConfigChangeResult applyConfigurationChange(final BackendVLVIndexCfg cfg)
{
try
{
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()))
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)
{
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);
newSortKeys = parseSortKeys(cfg.getSortOrder(), ccr);
ccr.setAdminActionRequired(true);
}

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

if (ccr.adminActionRequired())
final boolean requiresRebuild = ccr.adminActionRequired();
if (requiresRebuild)
{
trusted = false;
// 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)
Expand Down
Loading
Loading