diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java index 11fa660077..92c909832c 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java @@ -711,7 +711,8 @@ private WriteableTransaction newWriteableTransaction(Transaction txn) * @throws ConfigException * if memory cannot be reserved */ - JEStorage(final JEBackendCfg cfg, ServerContext serverContext) throws ConfigException + // Public as PDBStorage's is: a pluggable backend test which runs the same case over both storages builds them. + public JEStorage(final JEBackendCfg cfg, ServerContext serverContext) throws ConfigException { this.serverContext = serverContext; backendDirectory = getBackendDirectory(cfg); diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java index 055fdd6240..fa7e27e0a4 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/AttributeIndex.java @@ -43,6 +43,7 @@ import org.forgerock.opendj.ldap.Assertion; import org.forgerock.opendj.ldap.ByteSequence; import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.DN; import org.forgerock.opendj.ldap.DecodeException; import org.forgerock.opendj.ldap.schema.AttributeType; import org.forgerock.opendj.ldap.schema.MatchingRule; @@ -448,6 +449,103 @@ void open(WriteableTransaction txn, boolean createOnDemand) throws StorageRuntim config.addChangeListener(this); } + /** + * Drops whatever an index of the same name left behind for the trees this index, which the + * configuration is adding, is about to open. + *

+ * The name of an index tree is a pure function of the base DN, the attribute and the index id, and + * {@link #open} creates a tree only where there is none, so an index added for an attribute + * another index served reopens exactly the trees that one left behind - with their content and + * with the TRUSTED flag their {@code state} records carry. Neither is this index's: what those + * trees hold is what the backend was told before the configuration stopped naming them, every + * entry written in between is missing from it, and TRUSTED has searches answer out of it all the + * same. A rebuild regenerates all of it and nothing else is lost with it, so it is dropped here + * rather than adopted, which leaves this index where any other index added to a backend holding + * entries starts: empty, untrusted and asking to be rebuilt (#990). + *

+ * Only the trees of the index ids this configuration declares are looked at. A tree of an id it + * does not name is opened by nothing and answers nothing, and the first configuration which + * declares that id again drops it the same way. + *

+ * This must run in a write of its own, committed before the write which opens the index. On JE + * deleting a tree write-locks the record of its name until the transaction commits, while opening + * a tree - which {@code JEStorage} does under a transaction of its own - asks for a read lock on + * that record and waits for it without limit: no cycle, so the deadlock detector is silent, and + * the configuration change never returns. + *

+ * What this answers, the caller reports once every write of its change is over, whichever way + * they went - and when the write this ran in fails at its commit as well, although on JE and PDB + * that failure rolls the drop back. The report then overstates what happened: the trees are still + * there, the configuration entry is already written ({@code ConfigurationHandler} writes it before + * it notifies any listener), and the next open of the backend adopts them with their TRUSTED + * flag; the rebuild the report asks for is what puts that right. On JDBC the DROP has committed on + * its own before that commit failed, the record is back over a table which is gone, and the report + * is the only trace of it. Reported from a flag copied once the write has returned instead, the + * JE and PDB reports would be exact and the JDBC one silent, in the one case this method is for. + * + * @param txn a non null transaction + * @return true if a tree was dropped; a record deleted on its own discards nothing + * @throws StorageRuntimeException if an error occurs in the storage + */ + boolean dropLeftovers(WriteableTransaction txn) throws StorageRuntimeException + { + boolean dropped = false; + for (Index index : indexIdToIndexes.values()) + { + dropped |= dropLeftoversOf(txn, index); + } + return dropped; + } + + /** + * Drops the tree an index about to be opened would adopt, and the {@code state} record which goes + * with it. + *

+ * The record can outlive the tree on its own: on JDBC a tree is dropped by DDL which commits of + * its own accord while the record is deleted by the transaction, so a rollback in between leaves + * the record over a tree which is gone, and the index opened next is created empty and read back + * as trusted. It is therefore taken out whether a tree was found for it or not - but a record + * deleted on its own is not reported as discarded content, since none was. + *

+ * The tree is asked for through the transaction rather than through a list of the trees read + * beforehand: on JDBC that list borrows a connection of its own, which a transaction already + * holding one of the same pool must not ask for, while {@code treeExists} asks the transaction's + * own; on Cassandra the list is not implemented and answers nothing, while {@code treeExists} + * finds the partition; and a replayed attempt then sees what is there when it runs, not what was + * there before the first attempt. + *

+ * No search can be reading what is dropped here, so this does not take the exclusive lock + * {@link #deleteIndex} takes: an index which is only being added is in no map a search reaches, + * and the trees it would have adopted are named by nothing until it opens them. The add listener + * refuses an index for an attribute type which is already indexed, so that no live index is + * reached through another of the attribute's names or its OID. + * + * @return true if a tree was dropped + */ + private boolean dropLeftoversOf(WriteableTransaction txn, Index index) + { + if (txn.treeExists(index.getName())) + { + // Deletes the state record along with the tree. + entryContainer.deleteTree(txn, index); + return true; + } + state.deleteRecord(txn, index.getName()); + return false; + } + + /** + * Tells the operator that trees left behind were discarded rather than adopted, and puts it in the + * error log as well: the session which submitted the change ends, and what a backend was left + * holding has to be findable afterwards. + */ + static void reportDiscardedLeftovers(ConfigChangeResult ccr, Object indexName, DN baseDN) + { + final LocalizableMessage message = WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.get(indexName, baseDN); + ccr.addMessage(message); + logger.warn(message); + } + @Override public void close() { @@ -463,6 +561,15 @@ AttributeType getAttributeType() return config.getAttribute(); } + /** + * Get the configuration of this attribute index. + * @return The configuration this attribute index is currently applying. + */ + BackendIndexCfg getConfiguration() + { + return config; + } + public CryptoSuite getCryptoSuite() { return cryptoSuite; @@ -907,6 +1014,12 @@ public synchronized ConfigChangeResult applyConfigurationChange(final BackendInd { final ConfigChangeResult ccr = new ConfigChangeResult(); final IndexingOptions newIndexingOptions = new IndexingOptionsImpl(newConfiguration.getSubstringLength()); + // Drop what an earlier index left behind for the added ids, in a write of its own: the drop and the open + // must not share a transaction, see dropLeftovers(). discarded is filled by that write and reported from + // the finally below: every attempt fills it afresh, so a replayed attempt repeats nothing, and the report + // is not skipped when the write which opens the added indexes - or a later write of this change - throws + // after it, nor when the drop write fails at its own commit, for the reason dropLeftovers() gives. + final List discarded = new ArrayList<>(); try { final Map newIndexIdToIndexes = buildIndexes(entryContainer, state, newConfiguration, @@ -945,6 +1058,27 @@ public synchronized ConfigChangeResult applyConfigurationChange(final BackendInd ccr.addMessage(rebuildMessage); } + // A change which adds no index has nothing to drop, and opens no transaction for it - as the + // write which untrusts an index below opens none when there is nothing to untrust. + if (!addedIndexes.isEmpty()) + { + entryContainer.getRootContainer().getStorage().write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + discarded.clear(); + for (MatchingRuleIndex addedIndex : addedIndexes.values()) + { + if (dropLeftoversOf(txn, addedIndex)) + { + discarded.add(addedIndex); + } + } + } + }); + } + // Open added indexes *before* adding them to indexIdToIndexes final List addedIndexesToRebuild = new ArrayList<>(); entryContainer.getRootContainer().getStorage().write(new WriteOperation() @@ -1044,6 +1178,13 @@ public void run(WriteableTransaction txn) throws Exception ccr.setAdminActionRequired(true); ccr.addMessage(message); } + finally + { + for (MatchingRuleIndex index : discarded) + { + reportDiscardedLeftovers(ccr, index.getName(), entryContainer.getBaseDN()); + } + } return ccr; } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java index 70bf822ca1..4c6d71b47b 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java @@ -181,6 +181,12 @@ private class AttributeIndexCfgManager implements @Override public boolean isConfigurationAddAcceptable(final BackendIndexCfg cfg, List unacceptableReasons) { + final LocalizableMessage alreadyIndexed = alreadyIndexedBy(cfg); + if (alreadyIndexed != null) + { + unacceptableReasons.add(alreadyIndexed); + return false; + } try { newAttributeIndex(cfg, null); @@ -193,14 +199,56 @@ public boolean isConfigurationAddAcceptable(final BackendIndexCfg cfg, List + * The map is keyed by the attribute type, which every one of the attribute's names and its OID resolve to, + * while the configuration entry is named by whichever of them was typed. An index declared under another of + * them - commonName or 2.5.4.3 for cn - names the very trees the live index serves, and the add would drop + * them as left behind by an index which is gone. + */ + private LocalizableMessage alreadyIndexedBy(final BackendIndexCfg cfg) + { + final AttributeIndex existing = attrIndexMap.get(cfg.getAttribute()); + if (existing == null) + { + return null; + } + return ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED.get(cfg.getAttribute().getNameOrOID(), getBaseDN(), + existing.getConfiguration().dn()); + } + @Override public ConfigChangeResult applyConfigurationAdd(final BackendIndexCfg cfg) { final ConfigChangeResult ccr = new ConfigChangeResult(); + // Refused by isConfigurationAddAcceptable() before the configuration is written; asked again here since what + // follows drops the trees of the index it would have found. + final LocalizableMessage alreadyIndexed = alreadyIndexedBy(cfg); + if (alreadyIndexed != null) + { + ccr.setResultCode(ResultCode.UNWILLING_TO_PERFORM); + ccr.addMessage(alreadyIndexed); + return ccr; + } + // Dropped in a write of its own, committed before the write which opens the index: the two must not + // share a transaction, see AttributeIndex.dropLeftovers(). discarded is filled by that write and reported + // from a finally below: every attempt fills it afresh, so a replayed attempt repeats nothing, and the + // report is not skipped when the write which opens the index throws after it, nor when the drop write + // fails at its own commit - for the reason AttributeIndex.dropLeftovers() gives. + final AtomicBoolean discarded = new AtomicBoolean(); try { final CryptoSuite cryptoSuite = newCryptoSuite(cfg.isConfidentialityEnabled()); final AttributeIndex index = newAttributeIndex(cfg, cryptoSuite); + storage.write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + discarded.set(index.dropLeftovers(txn)); + } + }); final AtomicBoolean trusted = new AtomicBoolean(); storage.write(new WriteOperation() { @@ -229,6 +277,13 @@ public void run(WriteableTransaction txn) throws Exception ccr.setResultCode(DirectoryServer.getCoreConfigManager().getServerErrorResultCode()); ccr.addMessage(LocalizableMessage.raw(e.getLocalizedMessage())); } + finally + { + if (discarded.get()) + { + AttributeIndex.reportDiscardedLeftovers(ccr, cfg.getAttribute().getNameOrOID(), getBaseDN()); + } + } return ccr; } @@ -306,8 +361,21 @@ public boolean isConfigurationAddAcceptable(BackendVLVIndexCfg cfg, List built = new AtomicReference<>(); final AtomicBoolean trusted = new AtomicBoolean(); storage.write(new WriteOperation() @@ -343,6 +411,13 @@ public void run(WriteableTransaction txn) throws Exception ccr.setResultCode(DirectoryServer.getCoreConfigManager().getServerErrorResultCode()); ccr.addMessage(LocalizableMessage.raw(StaticUtils.stackTraceToSingleLineString(e))); } + finally + { + if (discarded.get()) + { + AttributeIndex.reportDiscardedLeftovers(ccr, cfg.getName(), getBaseDN()); + } + } return ccr; } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java index 6288ddc346..223e4c0b5f 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java @@ -114,8 +114,8 @@ class VLVIndex extends AbstractTree implements ConfigurationChangeListener + * What they hold is what the backend was told before the configuration stopped naming them, and no + * entry written in between is in it; a rebuild regenerates all of it. See + * {@link AttributeIndex#dropLeftovers} for why it is dropped rather than adopted (#990), and for + * why this must run in a write of its own, committed before the one which builds and opens the + * index: the constructor then reads its flag out of a record which is gone, and finds none. + * + * @param txn a non null transaction + * @param entryContainer the entry container the index is being added to + * @param state the tree holding the index flags + * @param indexName the name of the VLV index being added + * @return true if a tree was dropped; a record deleted on its own discards nothing + * @throws StorageRuntimeException if an error occurs in the storage + */ + static boolean dropLeftovers(WriteableTransaction txn, EntryContainer entryContainer, State state, String indexName) + throws StorageRuntimeException + { + final TreeName name = treeNameOf(entryContainer, indexName); + final TreeName counterName = counterTreeNameOf(entryContainer, indexName); + boolean dropped = false; + // Each of the two is asked for on its own: deleting a tree which is not there fails on PersistIt, + // and a change which stopped halfway can have left one of them without the other. + if (txn.treeExists(counterName)) + { + txn.deleteTree(counterName); + dropped = true; + } + if (txn.treeExists(name)) + { + txn.deleteTree(name); + dropped = true; + } + // The record can outlive the trees: see AttributeIndex.dropLeftoversOf. + state.deleteRecord(txn, name); + return dropped; + } + @Override void beforeDelete(WriteableTransaction txn) throws StorageRuntimeException { diff --git a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties index a897dc4cb0..f4aa8439e1 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1136,23 +1136,37 @@ ERR_CONFIG_INDEX_DELETE_FAILED_624=The index for attribute '%s' of backend base removed from the configuration, but the trees holding its data could not be deleted: %s. Those \ trees may still hold the data of that index, in whole or in part, and no configuration names them \ any more, so nothing will delete them and nothing will keep them up to date. An index created \ - again for that attribute with the same settings adopts those trees and is considered trusted over \ - their stale content, so it must be rebuilt before it is used + again for that attribute while this backend is open discards those trees and starts empty, asking \ + to be rebuilt as any index added to a backend holding entries does; one created while this \ + backend is disabled adopts them at the next open and is considered trusted over their stale \ + content, so it must be rebuilt before it is used ERR_CONFIG_VLV_INDEX_DELETE_FAILED_625=The VLV index '%s' of backend base DN '%s' was removed from \ the configuration, but the trees holding its data could not be deleted: %s. Those trees may still \ hold the data of that index, in whole or in part, and no configuration names them any more, so \ nothing will delete them and nothing will keep them up to date. A VLV index created again under \ - the same name adopts those trees and is considered trusted over their stale content, so it must \ - be rebuilt before it is used + the same name while this backend is open discards those trees and starts empty, asking to be \ + rebuilt as any VLV index added to a backend holding entries does; one created while this backend \ + is disabled adopts them at the next open and is considered trusted over their stale content, so \ + it must be rebuilt before it is used ERR_CONFIG_INDEX_CHANGE_FAILED_626=The configuration of the index for attribute '%s' of backend \ base DN '%s' could not be applied in full: %s. What this index holds and what its configuration \ declares may no longer agree, so this index must be rebuilt before it is used, after this backend \ has been disabled and enabled again so that every tree the stored configuration declares is named \ once more. The trees of the index types this change removed may still hold their data while no \ configuration names them any more, so nothing will delete them and nothing will keep them up to \ - date; an index type declared again for that attribute adopts those trees and is considered trusted \ - over their stale content, and must be rebuilt too + date; an index type declared again for that attribute while this backend is open discards those \ + trees and starts empty, asking to be rebuilt, while one declared again while this backend is \ + disabled adopts them at the next open and is considered trusted over their stale content, and \ + must be rebuilt too ERR_CONFIG_BACKEND_DATA_CHANGE_FAILED_627=The compression, encoding and encryption settings of \ backend base DN '%s' could not be applied in full: %s. Its entries and its indexes may no longer \ be encoded under the same settings; disabling and enabling this backend builds both from the \ stored configuration again +WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES_628=Index %s of backend base DN '%s' was created over the trees an \ + index of the same name left behind, which no configuration named while this backend went on taking writes. \ + What they hold is not what this index holds - every entry changed while nothing named them is missing from \ + it - so their content has been discarded rather than adopted, and this index has been created empty. Rebuild \ + it before it is used +ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED_629=Attribute %s of backend base DN '%s' is already indexed by %s. \ + An attribute type is indexed once, whichever of its names or its OID the index is declared by, so change that \ + index instead of adding another one for the same attribute diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ConfigChangeGivesUpTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ConfigChangeGivesUpTest.java index 538656333d..17ffcc7a6e 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ConfigChangeGivesUpTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ConfigChangeGivesUpTest.java @@ -24,6 +24,8 @@ import static org.opends.messages.BackendMessages.ERR_CONFIG_INDEX_CHANGE_FAILED; import static org.opends.messages.BackendMessages.ERR_CONFIG_INDEX_DELETE_FAILED; import static org.opends.messages.BackendMessages.ERR_CONFIG_VLV_INDEX_DELETE_FAILED; +import static org.opends.messages.BackendMessages.NOTE_INDEX_ADD_REQUIRES_REBUILD; +import static org.opends.messages.BackendMessages.WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES; import static org.opends.server.util.CollectionUtils.newTreeSet; import java.util.HashSet; @@ -174,10 +176,10 @@ public void aVlvIndexDeletionWhichGivesUpAsksForAnAdministrativeAction() throws } /** - * An index change is applied by three writes, and the second one deletes the trees of the - * indexes the new configuration no longer asks for. When it gives up, those trees are still - * there, so the index has to go on naming them: an index taken out of the map while its trees - * survive is an index nothing maintains and nothing deletes. + * An index change which adds no index type is applied by three writes, and the second one + * deletes the trees of the indexes the new configuration no longer asks for. When it gives up, + * those trees are still there, so the index has to go on naming them: an index taken out of the + * map while its trees survive is an index nothing maintains and nothing deletes. */ @Test public void anIndexChangeWhichGivesUpDeletingGoesOnNamingWhatItCouldNotDelete() throws Exception @@ -210,7 +212,7 @@ public void anIndexChangeWhichGivesUpDeletingGoesOnNamingWhatItCouldNotDelete() } /** - * The third write is what untrusts the indexes which stay when the new configuration raises + * The last write is what untrusts the indexes which stay when the new configuration raises * their entry limit, and the limit itself is applied to them only once it has committed. When it * gives up, the configuration must not be published either, or the index claims settings which * were never applied to it. What it declares is read here through the index types it names, the @@ -227,9 +229,10 @@ public void anIndexChangeWhichGivesUpUpdatingDoesNotPublishWhatItCouldNotApply() final Set indexIdsBefore = new HashSet<>(index.getNameToIndexes().keySet()); // A presence index is added and the entry limit of the ones which stay is raised, so that - // all three writes have work to do and the third is the one which gives up: a lowered limit - // untrusts nothing and opens no third write. - backend.storage.giveUpOnWrite(3); + // all four writes have work to do and the fourth is the one which gives up: an added index + // type is what opens the write which drops its leftovers first (#990), and a lowered limit + // untrusts nothing and opens no last write. + backend.storage.giveUpOnWrite(4); final ConfigChangeResult ccr = index.applyConfigurationChange( indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING, IndexType.PRESENCE), 5000)); @@ -400,14 +403,16 @@ public void anIndexChangeWhichSucceedsPublishesEveryPartOfIt() throws Exception /** * Pins what {@code ERR_CONFIG_INDEX_DELETE_FAILED} tells the operator: an index created again for - * the same attribute adopts the trees a failed deletion left behind, and is trusted over their - * stale content without a word about rebuilding it. This is not the behaviour being asked for - * here - it is the behaviour that message describes (#990). When it is fixed, this test fails and - * the message has to be rewritten, rather than quietly becoming untrue. Its VLV counterpart - * says the same of {@code VLVIndex.afterOpen}, which nothing here holds. + * the same attribute while the backend is open discards the trees a failed deletion left behind + * rather than adopting them, starts empty and untrusted, and asks to be rebuilt - the way any + * index added to a backend holding entries does - and says that content was discarded to get + * there (#990). Before that fix the index adopted those trees and was trusted over their stale + * content without a word about rebuilding it, which is what the message used to describe. The + * adoption the message still describes, of an index created while the backend is disabled, runs + * through {@code EntryContainer.open}, which nothing here holds. */ @Test - public void anIndexCreatedAgainAdoptsTheTreesAFailedDeletionLeftBehind() throws Exception + public void anIndexCreatedAgainDiscardsTheTreesAFailedDeletionLeftBehind() throws Exception { final GivingUpBackend backend = openBackend(); try @@ -426,11 +431,14 @@ public void anIndexCreatedAgainAdoptsTheTreesAFailedDeletionLeftBehind() throws assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); assertThat(treesOf(ec.getAttributeIndex(cnType))) - .as("the trees the index created again holds").isEqualTo(indexTrees); + .as("the trees the index created again names, under the names the deletion could not remove") + .isEqualTo(indexTrees); assertThat(ec.getAttributeIndex(cnType).isTrusted()) - .as("an index trusted over the content of the trees it adopted").isTrue(); - assertThat(ccr.getMessages()) - .as("nothing tells the operator this index has to be rebuilt").isEmpty(); + .as("an index trusted over the content of trees it did not fill").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr)) + .as("the rebuild this index asks for, and the content discarded to get there") + .contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal(), WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); } finally { diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexAddedOverLeftoverTreesTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexAddedOverLeftoverTreesTest.java new file mode 100644 index 0000000000..d83d1c1130 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/IndexAddedOverLeftoverTreesTest.java @@ -0,0 +1,1301 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.opends.server.backends.pluggable; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.opends.messages.BackendMessages.ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED; +import static org.opends.messages.BackendMessages.NOTE_INDEX_ADD_REQUIRES_REBUILD; +import static org.opends.messages.BackendMessages.WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES; +import static org.opends.server.backends.pluggable.State.IndexFlag.TRUSTED; +import static org.opends.server.backends.pluggable.SuffixContainer.STATE_INDEX_NAME; +import static org.opends.server.util.CollectionUtils.newTreeSet; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.SortedSet; + +import org.forgerock.i18n.LocalizableMessage; +import org.forgerock.opendj.config.server.ConfigChangeResult; +import org.forgerock.opendj.config.server.ConfigException; +import org.forgerock.opendj.config.server.ConfigurationAddListener; +import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.DN; +import org.forgerock.opendj.ldap.ResultCode; +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.ErrorLogPublisherCfg; +import org.forgerock.opendj.server.config.server.JEBackendCfg; +import org.forgerock.opendj.server.config.server.PDBBackendCfg; +import org.forgerock.opendj.server.config.server.PluggableBackendCfg; +import org.mockito.ArgumentCaptor; +import org.opends.messages.Severity; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.opends.server.backends.jeb.JEStorage; +import org.opends.server.backends.pdb.PDBStorage; +import org.opends.server.backends.pluggable.AttributeIndex.MatchingRuleIndex; +import org.opends.server.backends.pluggable.spi.AccessMode; +import org.opends.server.backends.pluggable.spi.Cursor; +import org.opends.server.backends.pluggable.spi.Importer; +import org.opends.server.backends.pluggable.spi.ReadOperation; +import org.opends.server.backends.pluggable.spi.Storage; +import org.opends.server.backends.pluggable.spi.StorageStatus; +import org.opends.server.backends.pluggable.spi.TreeName; +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.ServerContext; +import org.opends.server.loggers.ErrorLogPublisher; +import org.opends.server.loggers.ErrorLogger; +import org.opends.server.types.BackupConfig; +import org.opends.server.types.BackupDirectory; +import org.opends.server.types.DirectoryException; +import org.opends.server.types.Entry; +import org.opends.server.types.RestoreConfig; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** + * Tests what a pluggable backend does with the trees of an index which is declared again after the + * configuration naming them is gone. The name of an index tree is a pure function of the base DN, + * the attribute and the index id, and opening an index creates its trees only if they are not + * there, so an index added for an attribute a previous index of the same name served reopens + * exactly the trees that one left behind - with their content and with the TRUSTED flag their + * {@code state} records still carry. Neither belongs to the index being added: what those trees + * hold is whatever the backend was told before the configuration stopped naming them, and every + * entry written in between is missing from it - see OpenDJ issue #990. + *

+ * The trees are left behind here by declaring them, filling them and then reopening the backend + * from a configuration which no longer names them, which is what an index deleted while the + * backend is disabled - or through an offline {@code dsconfig} - leaves behind: the entry + * container registers the listener which would have deleted the trees only while it is open + * ({@code EntryContainer}, its constructor and {@code close()}). A deletion the storage gives up on + * and a stop between the deletion of the trees and the commit of the configuration change leave the + * same state; #962 covers the first of those. + *

+ * Every case runs over PDB and over JE. The two lock differently: JE write-locks the name of a + * deleted tree until the transaction commits, and opening a tree of that name from the same + * transaction - which JE does under a transaction of its own - waits for it forever, so a drop and + * an open sharing one write never return there. The time limit is what makes that a failure rather + * than a hang; under Maven {@code TestListener} replaces it with {@code org.opends.test.timeout}. + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "pluggablebackend" }, sequential = true) +public class IndexAddedOverLeftoverTreesTest extends DirectoryServerTestCase +{ + private static final String BACKEND_ID = "IndexAddedOverLeftoverTreesTest"; + private static final DN BASE_DN = DN.valueOf("dc=b990,dc=com"); + private static final String VLV_INDEX_NAME = "b990vlv"; + private static final long HANG_TIMEOUT_MS = 60_000; + + /** The storages the cases run over, each in a directory of its own. */ + enum StorageKind + { + PDB + { + @Override + PluggableBackendCfg newBackendCfg(String dbDirectory) + { + final PDBBackendCfg cfg = mockCfg(PDBBackendCfg.class); + when(cfg.getDBDirectory()).thenReturn(dbDirectory); + when(cfg.getDBDirectoryPermissions()).thenReturn("755"); + when(cfg.getDBCacheSize()).thenReturn(0L); + when(cfg.getDBCachePercent()).thenReturn(20); + return cfg; + } + + @Override + Storage newStorage(PluggableBackendCfg cfg, ServerContext serverContext) throws ConfigException + { + return new PDBStorage((PDBBackendCfg) cfg, serverContext); + } + }, + JE + { + @Override + PluggableBackendCfg newBackendCfg(String dbDirectory) + { + final JEBackendCfg cfg = mockCfg(JEBackendCfg.class); + when(cfg.getDBDirectory()).thenReturn(dbDirectory); + when(cfg.getDBDirectoryPermissions()).thenReturn("755"); + when(cfg.getDBCacheSize()).thenReturn(0L); + when(cfg.getDBCachePercent()).thenReturn(20); + when(cfg.getDBNumCleanerThreads()).thenReturn(2); + when(cfg.getDBNumLockTables()).thenReturn(63); + return cfg; + } + + @Override + Storage newStorage(PluggableBackendCfg cfg, ServerContext serverContext) throws ConfigException + { + return new JEStorage((JEBackendCfg) cfg, serverContext); + } + }; + + /** A configuration of this storage, with its files under the given directory. */ + abstract PluggableBackendCfg newBackendCfg(String dbDirectory); + + abstract Storage newStorage(PluggableBackendCfg cfg, ServerContext serverContext) throws ConfigException; + } + + private ServerContext serverContext; + private AttributeType cnType; + + @BeforeClass + public void startServer() throws Exception + { + TestCaseUtils.startServer(); + serverContext = TestCaseUtils.getServerContext(); + cnType = serverContext.getSchema().getAttributeType("cn"); + } + + @DataProvider(name = "storages") + public Object[][] storages() + { + return new Object[][] { { StorageKind.PDB }, { StorageKind.JE } }; + } + + /** + * A test which fails before it finalizes its backend leaves the base DN behind in the server wide + * registry, where it would outlive the test and break the next one to use it. + */ + @AfterMethod + public void deregisterLeftoverBaseDN() + { + try + { + serverContext.getBackendConfigManager().deregisterBaseDN(BASE_DN); + } + catch (Exception alreadyGone) + { + // Which is what a test that finalized its backend leaves behind. + } + } + + /** + * The index the add opens is not the index whose trees are still there: it has indexed none of + * the entries, so it must be untrusted and the operator must be told to rebuild it, exactly as + * for any other index added to a backend which already holds entries - and told, in the change + * result and in the error log, that content was discarded to get there. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedOverTheTreesLeftBehindIsNotTrusted(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + final ErrorLogPublisher errorLog = registerErrorLogCapture(); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + assertThat(backend.getRootContainer().getStorage().listTrees()) + .as("the trees no configuration names any more").containsAll(backend.leftoverIndexTrees); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ec.getAttributeIndex(cnType).isTrusted()) + .as("an index trusted over the content of trees it did not fill").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal(), + WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(renderedOf(ccr.getMessages())).as("the index and the base DN the report names") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.get("cn", BASE_DN).toString()); + assertThat(warningOrdinalsLoggedTo(errorLog)) + .as("the discard reported in the error log, not only in the change result") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + } + finally + { + ErrorLogger.getInstance().removeLogPublisher(errorLog); + backend.finalizeBackend(); + } + } + + /** + * The flag alone does not make the answers right again: {@code DefaultIndex.get} hands back what + * a key holds whenever it holds anything, whatever the flag says, and only a key with nothing + * behind it answers "undefined" and sends the search to the entries. So the content of the + * adopted trees has to go, or every key they hold goes on answering with the entries of another + * index and misses everything written since. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedOverTheTreesLeftBehindDoesNotAnswerWithTheirContent(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + final Map keysHeld = keysHeldBy(storage, backend.leftoverIndexTrees); + assertThat(keysHeld).as("the keys the trees left behind hold").isNotEmpty(); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ordinalsOf(ccr.getMessages())).contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + final AttributeIndex index = ec.getAttributeIndex(cnType); + for (final MatchingRuleIndex opened : index.getNameToIndexes().values()) + { + final ByteString keyHeld = keysHeld.get(opened.getName()); + if (keyHeld != null) + { + assertThat(answers(storage, opened, keyHeld)) + .as("a key of " + opened.getName() + " answered out of the tree left behind").isFalse(); + } + } + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * The trees can be gone while their {@code state} records are not: on JDBC the drop of a table is + * DDL which commits of its own accord, while the {@code state.deleteRecord} of the same + * {@code closeAndDelete} belongs to the transaction, so a rollback after the drop restores the + * records over tables which are already gone. The index added next creates empty trees and reads + * TRUSTED out of those records - an empty index which every search believes. Nothing is + * discarded on this road, so nothing says it was: the record goes, and the rebuild is asked for. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedOverAStateRecordWhoseTreesAreGoneIsNotTrusted(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + storage.write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + for (final TreeName leftover : backend.leftoverIndexTrees) + { + txn.deleteTree(leftover); + } + } + }); + for (final TreeName leftover : backend.leftoverIndexTrees) + { + assertThat(persistedFlags(backend, leftover)) + .as("the state record left over the tree which is gone").contains(TRUSTED); + } + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ec.getAttributeIndex(cnType).isTrusted()) + .as("an empty index trusted over a state record which outlived its trees").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal()); + assertThat(ordinalsOf(ccr.getMessages())).as("content reported as discarded where no tree was") + .doesNotContain(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * {@code dropLeftovers} folds each id's result with {@code dropped |= ...} rather than the last one + * assigned; the two only disagree when the ids themselves do, which every other case avoids by giving + * all of them a leftover or none. Deleting one of the two trees {@code leaveTreesBehind} left - the + * cn index has EQUALITY and SUBSTRING - ahead of the add leaves the other one real, so the fold and + * the assignment can differ on whether the change discarded anything. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedOverOnlyOneOfTwoLeftoverTreesIsReportedDiscarded(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final Storage storage = backend.getRootContainer().getStorage(); + final TreeName oneOfTwo = backend.leftoverIndexTrees.iterator().next(); + storage.write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + txn.deleteTree(oneOfTwo); + } + }); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())) + .as("the other of the two ids still had a real tree to discard") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * A VLV index adopts what it left behind in the same way, in its tree and in its counter. Both are + * dropped, and so is the {@code state} record the next open of the backend would read the flag + * out of. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aVlvIndexAddedOverTheTreesLeftBehindIsNotTrusted(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + final ErrorLogPublisher errorLog = registerErrorLogCapture(); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + assertThat(storage.listTrees()) + .as("the VLV trees no configuration names any more") + .contains(backend.leftoverVlvTree, backend.leftoverVlvCounterTree); + assertThat(holdsAnything(storage, backend.leftoverVlvTree)).as("the VLV tree left behind").isTrue(); + assertThat(holdsAnything(storage, backend.leftoverVlvCounterTree)).as("the VLV counter left behind").isTrue(); + assertThat(persistedFlags(backend, backend.leftoverVlvTree)).contains(TRUSTED); + + final ConfigChangeResult ccr = vlvIndexAddListener(backend).applyConfigurationAdd(backend.vlvIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ec.getVLVIndex(VLV_INDEX_NAME).isTrusted()) + .as("a VLV index trusted over the content of trees it did not fill").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal(), + WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(renderedOf(ccr.getMessages())).as("the index and the base DN the report names") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.get(VLV_INDEX_NAME, BASE_DN).toString()); + assertThat(warningOrdinalsLoggedTo(errorLog)) + .as("the discard reported in the error log, not only in the change result") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(holdsAnything(storage, backend.leftoverVlvTree)) + .as("the VLV tree left behind still holds its content").isFalse(); + assertThat(holdsAnything(storage, backend.leftoverVlvCounterTree)) + .as("the VLV counter left behind still holds its count").isFalse(); + assertThat(persistedFlags(backend, backend.leftoverVlvTree)) + .as("the TRUSTED record outlived the add").doesNotContain(TRUSTED); + } + finally + { + ErrorLogger.getInstance().removeLogPublisher(errorLog); + backend.finalizeBackend(); + } + } + + /** + * The state record can outlive the VLV tree and its counter the same way it outlives an + * attribute index's tree + * ({@link #anIndexAddedOverAStateRecordWhoseTreesAreGoneIsNotTrusted(StorageKind)}): deleting + * them by hand exercises the two {@code treeExists} guards in + * {@code VLVIndex.dropLeftovers} on their false arm, which the fixture of the case above never + * reaches. On JE, {@code deleteTree} of a tree which is not there is silent, so a guard removed + * or inverted would say content was discarded where none was. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aVlvIndexAddedOverAStateRecordWhoseTreesAreGoneIsNotTrusted(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + storage.write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + txn.deleteTree(backend.leftoverVlvTree); + txn.deleteTree(backend.leftoverVlvCounterTree); + } + }); + assertThat(persistedFlags(backend, backend.leftoverVlvTree)) + .as("the state record left over the VLV tree which is gone").contains(TRUSTED); + + final ConfigChangeResult ccr = vlvIndexAddListener(backend).applyConfigurationAdd(backend.vlvIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(ec.getVLVIndex(VLV_INDEX_NAME).isTrusted()) + .as("an empty VLV index trusted over a state record which outlived its trees").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal()); + assertThat(ordinalsOf(ccr.getMessages())).as("content reported as discarded where no tree was") + .doesNotContain(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * An index type declared again for an attribute which is still indexed goes through + * {@code AttributeIndex.applyConfigurationChange} rather than through the add listener, and opens + * its tree the same way. Only the tree of the id declared again goes: the tree of the id the index + * was serving all along keeps what it holds. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexTypeAddedOverTheTreeLeftBehindIsNotTrusted(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind, indexCfg(newTreeSet(IndexType.EQUALITY))); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + final AttributeIndex index = ec.getAttributeIndex(cnType); + assertThat(index.isIndexed(IndexType.SUBSTRING)) + .as("the index type this configuration no longer names").isFalse(); + final Map keysHeld = keysHeldBy(storage, backend.leftoverIndexTrees); + // The index ids are the matching rules' - the one the index serves now is the equality one. + final Map servedBefore = new HashMap<>(index.getNameToIndexes()); + assertThat(servedBefore).hasSize(1); + final MatchingRuleIndex equality = servedBefore.values().iterator().next(); + final ByteString equalityKey = keysHeld.get(equality.getName()); + assertThat(equalityKey).as("a key the live equality tree holds").isNotNull(); + + final ConfigChangeResult ccr = + index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING))); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(index.isIndexed(IndexType.SUBSTRING)).isTrue(); + assertThat(index.isTrusted()) + .as("an index type trusted over the content of the tree it did not fill").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal(), + WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + final Map added = new HashMap<>(index.getNameToIndexes()); + added.keySet().removeAll(servedBefore.keySet()); + assertThat(added).hasSize(1); + final MatchingRuleIndex substring = added.values().iterator().next(); + // Named by its tree on this road, where the add listener names the index by its attribute. + assertThat(renderedOf(ccr.getMessages())).as("the tree and the base DN the report names") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.get(substring.getName(), BASE_DN).toString()); + assertThat(keysHeld).as("the substring tree left behind held a key").containsKey(substring.getName()); + assertThat(answers(storage, substring, keysHeld.get(substring.getName()))) + .as("a substring key answered out of the tree left behind").isFalse(); + assertThat(answers(storage, equality, equalityKey)) + .as("the live equality tree went with the leftover").isTrue(); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * What the drop write found is reported from a {@code finally}, so a throw from the write which + * opens the index - after the drop has committed and the content is gone for good - does not + * swallow it: the change fails, and the change result and the error log still say what was + * discarded. Neither storage engine can be made to throw on its own, so the failure is injected + * through {@link RefusingStorage}: the drop write goes through, the open write is refused before + * it runs. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aDiscardIsStillReportedWhenTheWriteWhichOpensTheIndexFails(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + final ErrorLogPublisher errorLog = registerErrorLogCapture(); + try + { + final Storage storage = backend.getRootContainer().getStorage(); + final int writesBefore = backend.storage.writes(); + backend.storage.refuseWrite(2); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(backend.storage.writes()).as("the refused write was the second, and the last one made") + .isEqualTo(writesBefore + 2); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the discard the failed change swallowed") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(warningOrdinalsLoggedTo(errorLog)).contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + for (final TreeName leftover : backend.leftoverIndexTrees) + { + assertThat(storage. read(txn -> txn.treeExists(leftover))) + .as("the tree the report says was discarded: " + leftover).isFalse(); + } + } + finally + { + ErrorLogger.getInstance().removeLogPublisher(errorLog); + backend.finalizeBackend(); + } + } + + /** The VLV road reports from a {@code finally} the same way. */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aVlvDiscardIsStillReportedWhenTheWriteWhichBuildsTheIndexFails(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + final ErrorLogPublisher errorLog = registerErrorLogCapture(); + try + { + final Storage storage = backend.getRootContainer().getStorage(); + final int writesBefore = backend.storage.writes(); + backend.storage.refuseWrite(2); + + final ConfigChangeResult ccr = vlvIndexAddListener(backend).applyConfigurationAdd(backend.vlvIndexCfg); + + assertThat(backend.storage.writes()).as("the refused write was the second, and the last one made") + .isEqualTo(writesBefore + 2); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the discard the failed change swallowed") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(warningOrdinalsLoggedTo(errorLog)).contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(storage. read(txn -> txn.treeExists(backend.leftoverVlvTree))) + .as("the VLV tree the report says was discarded").isFalse(); + assertThat(storage. read(txn -> txn.treeExists(backend.leftoverVlvCounterTree))) + .as("the VLV counter the report says was discarded").isFalse(); + } + finally + { + ErrorLogger.getInstance().removeLogPublisher(errorLog); + backend.finalizeBackend(); + } + } + + /** + * And so does {@code AttributeIndex.applyConfigurationChange}, whose drop write is the first of + * the writes a change which adds an index type makes, and whose open write is the second. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexTypeDiscardIsStillReportedWhenTheWriteWhichOpensItFails(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind, indexCfg(newTreeSet(IndexType.EQUALITY))); + final ErrorLogPublisher errorLog = registerErrorLogCapture(); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + final AttributeIndex index = ec.getAttributeIndex(cnType); + final Map keysHeld = keysHeldBy(storage, backend.leftoverIndexTrees); + final MatchingRuleIndex equality = index.getNameToIndexes().values().iterator().next(); + final Set substringTrees = new HashSet<>(backend.leftoverIndexTrees); + substringTrees.remove(equality.getName()); + assertThat(substringTrees).hasSize(1); + final int writesBefore = backend.storage.writes(); + backend.storage.refuseWrite(2); + + final ConfigChangeResult ccr = + index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING))); + + assertThat(backend.storage.writes()).as("the refused write was the second, and the last one made") + .isEqualTo(writesBefore + 2); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the discard the failed change swallowed") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(warningOrdinalsLoggedTo(errorLog)).contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + final TreeName substringTree = substringTrees.iterator().next(); + assertThat(storage. read(txn -> txn.treeExists(substringTree))) + .as("the tree the report says was discarded").isFalse(); + assertThat(index.isIndexed(IndexType.SUBSTRING)).as("an index type the failed change declared").isFalse(); + assertThat(answers(storage, equality, keysHeld.get(equality.getName()))) + .as("the live equality tree went with the leftover").isTrue(); + } + finally + { + ErrorLogger.getInstance().removeLogPublisher(errorLog); + backend.finalizeBackend(); + } + } + + /** + * The drop write can fail at its own commit, once it has run and answered - a disk which is full + * when the engine writes its log. JE and PDB roll the drop back with it: the trees are still + * there, and so is the TRUSTED record the next open of the backend reads. The report stands all + * the same, and says more than happened. It is kept that way on purpose: the configuration entry + * is already written when the listener runs, so what comes next is not another add but that + * open, which adopts the trees - and the rebuild the report asks for is what puts it right. On + * JDBC the same failure comes after a DROP which committed on its own, and the report is the only + * trace of it; a report made to wait for the commit would be silent there, in the one case the + * drop exists for. Pinned so that moving it behind the commit is red here rather than silent + * there. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aDiscardIsStillReportedWhenTheDropWriteFailsAtItsCommit(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final Storage storage = backend.getRootContainer().getStorage(); + final int writesBefore = backend.storage.writes(); + backend.storage.failWriteAfterItRan(1); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(backend.storage.writes()).as("the failed write was the first, and the last one made") + .isEqualTo(writesBefore + 1); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the report of a drop the engine rolled back") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + for (final TreeName leftover : backend.leftoverIndexTrees) + { + assertThat(holdsAnything(storage, leftover)).as("the drop of " + leftover + " rolled back with the write") + .isTrue(); + assertThat(persistedFlags(backend, leftover)).as("the record of " + leftover).contains(TRUSTED); + } + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * The VLV road keeps its report when the drop write fails at its commit, on the terms the case + * above gives: the tree and its counter are back, and so is the TRUSTED record. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aVlvDiscardIsStillReportedWhenTheDropWriteFailsAtItsCommit(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind); + try + { + final Storage storage = backend.getRootContainer().getStorage(); + final int writesBefore = backend.storage.writes(); + backend.storage.failWriteAfterItRan(1); + + final ConfigChangeResult ccr = vlvIndexAddListener(backend).applyConfigurationAdd(backend.vlvIndexCfg); + + assertThat(backend.storage.writes()).as("the failed write was the first, and the last one made") + .isEqualTo(writesBefore + 1); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the report of a drop the engine rolled back") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(holdsAnything(storage, backend.leftoverVlvTree)).as("the VLV tree rolled back with the write") + .isTrue(); + assertThat(holdsAnything(storage, backend.leftoverVlvCounterTree)) + .as("the VLV counter rolled back with the write").isTrue(); + assertThat(persistedFlags(backend, backend.leftoverVlvTree)).as("the record of the VLV tree").contains(TRUSTED); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * And so does {@code AttributeIndex.applyConfigurationChange}: the tree of the index type + * declared again is back with its TRUSTED record, the change has not been applied, and the + * report stands. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexTypeDiscardIsStillReportedWhenTheDropWriteFailsAtItsCommit(StorageKind kind) throws Exception + { + final LeftoverBackend backend = leaveTreesBehind(kind, indexCfg(newTreeSet(IndexType.EQUALITY))); + try + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + final AttributeIndex index = ec.getAttributeIndex(cnType); + final Map keysHeld = keysHeldBy(storage, backend.leftoverIndexTrees); + final MatchingRuleIndex equality = index.getNameToIndexes().values().iterator().next(); + final Set substringTrees = new HashSet<>(backend.leftoverIndexTrees); + substringTrees.remove(equality.getName()); + assertThat(substringTrees).hasSize(1); + final TreeName substringTree = substringTrees.iterator().next(); + final int writesBefore = backend.storage.writes(); + backend.storage.failWriteAfterItRan(1); + + final ConfigChangeResult ccr = + index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING))); + + assertThat(backend.storage.writes()).as("the failed write was the first, and the last one made") + .isEqualTo(writesBefore + 1); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).as("the report of a drop the engine rolled back") + .contains(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + assertThat(holdsAnything(storage, substringTree)).as("the substring tree rolled back with the write").isTrue(); + assertThat(persistedFlags(backend, substringTree)).as("the record of the substring tree").contains(TRUSTED); + assertThat(index.isIndexed(IndexType.SUBSTRING)).as("an index type the failed change declared").isFalse(); + assertThat(answers(storage, equality, keysHeld.get(equality.getName()))) + .as("the live equality tree went with the leftover").isTrue(); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * An index added to a backend which holds no entry has nothing to index and nothing to adopt, so + * it stays trusted and asks for nothing. Pins the upgrade {@code DefaultIndex.afterOpen} makes. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedToAnEmptyBackendIsTrustedAndAsksForNothing(StorageKind kind) throws Exception + { + final LeftoverBackend backend = openBackend(kind, true, null, null); + try + { + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(backend.getRootContainer().getEntryContainer(BASE_DN).getAttributeIndex(cnType).isTrusted()) + .as("an index of a backend with nothing to index").isTrue(); + assertThat(ccr.adminActionRequired()).isFalse(); + assertThat(ccr.getMessages()).isEmpty(); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * An index added to a backend which holds entries, with nothing left behind for it to adopt, is + * untrusted over trees it created empty and asks to be rebuilt. Pins what the fix has to leave + * alone: this is the outcome an index added over leftover trees has to reach as well, and nothing + * was discarded to reach it here. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void anIndexAddedToANonEmptyBackendAsksForARebuild(StorageKind kind) throws Exception + { + final LeftoverBackend backend = openBackend(kind, true, null, null); + try + { + addEntry(backend, "dn: " + BASE_DN, "objectClass: top", "objectClass: domain", "dc: b990"); + + final ConfigChangeResult ccr = indexAddListener(backend).applyConfigurationAdd(backend.cnIndexCfg); + + assertThat(ccr.getResultCode()).isEqualTo(ResultCode.SUCCESS); + assertThat(backend.getRootContainer().getEntryContainer(BASE_DN).getAttributeIndex(cnType).isTrusted()) + .as("an index which has indexed none of the entries").isFalse(); + assertThat(ccr.adminActionRequired()).isTrue(); + assertThat(ordinalsOf(ccr.getMessages())).contains(NOTE_INDEX_ADD_REQUIRES_REBUILD.ordinal()); + assertThat(ordinalsOf(ccr.getMessages())).as("content reported as discarded where nothing was left behind") + .doesNotContain(WARN_INDEX_ADD_DISCARDED_LEFTOVER_TREES.ordinal()); + } + finally + { + backend.finalizeBackend(); + } + } + + /** + * The entry container keys its indexes by the attribute type, which every name of the attribute + * and its OID resolve to, while a configuration entry is named by whichever of them was typed. An + * index declared for cn as commonName or as 2.5.4.3 is therefore a second index for an attribute + * which is already indexed, naming the very trees the live index serves - and those must not go + * the way of trees left behind. The add is refused instead, before the configuration is written + * and again where it would have dropped them. + */ + @Test(dataProvider = "storages", timeOut = HANG_TIMEOUT_MS) + public void aSecondIndexForAnAttributeAlreadyIndexedIsRefused(StorageKind kind) throws Exception + { + final LeftoverBackend backend = + openBackend(kind, true, indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING)), null); + try + { + addEntry(backend, "dn: " + BASE_DN, "objectClass: top", "objectClass: domain", "dc: b990"); + addEntry(backend, "dn: cn=live," + BASE_DN, "objectClass: top", "objectClass: organizationalRole", "cn: live"); + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final Storage storage = backend.getRootContainer().getStorage(); + final AttributeIndex live = ec.getAttributeIndex(cnType); + assertThat(live.isTrusted()).isTrue(); + final Map keysHeld = keysHeldBy(storage, treesOf(live)); + assertThat(keysHeld).as("the keys the live index holds").isNotEmpty(); + + // What was typed names the configuration entry; the attribute type it resolves to is the one already indexed. + final DN typedAs = DN.valueOf("ds-cfg-attribute=commonName,cn=Index," + backend.configuredWith.dn()); + final BackendIndexCfg declaredAgain = indexCfg(newTreeSet(IndexType.EQUALITY)); + when(declaredAgain.dn()).thenReturn(typedAs); + final ConfigurationAddListener listener = indexAddListener(backend); + + // An index for an attribute which is not indexed yet is still accepted - pins the refusing arm above + // against a mutant which refuses every add. + final BackendIndexCfg fresh = indexCfg(newTreeSet(IndexType.EQUALITY)); + when(fresh.getAttribute()).thenReturn(serverContext.getSchema().getAttributeType("sn")); + final List reasons = new ArrayList<>(); + assertThat(listener.isConfigurationAddAcceptable(fresh, reasons)).isTrue(); + assertThat(reasons).isEmpty(); + + assertThat(listener.isConfigurationAddAcceptable(declaredAgain, reasons)).isFalse(); + assertThat(ordinalsOf(reasons)).contains(ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED.ordinal()); + + final ConfigChangeResult ccr = listener.applyConfigurationAdd(declaredAgain); + assertThat(ccr.getResultCode()).isNotEqualTo(ResultCode.SUCCESS); + assertThat(ordinalsOf(ccr.getMessages())).contains(ERR_CONFIG_INDEX_ATTRIBUTE_ALREADY_INDEXED.ordinal()); + + assertThat(ec.getAttributeIndex(cnType)).as("the live index was replaced").isSameAs(live); + assertThat(live.isTrusted()).isTrue(); + for (final MatchingRuleIndex opened : live.getNameToIndexes().values()) + { + assertThat(answers(storage, opened, keysHeld.get(opened.getName()))) + .as("a key of " + opened.getName() + " the live index held before").isTrue(); + } + } + finally + { + backend.finalizeBackend(); + } + } + + private LeftoverBackend leaveTreesBehind(StorageKind kind) throws Exception + { + return leaveTreesBehind(kind, null); + } + + /** + * Opens a backend whose configuration names a cn index and a VLV index, fills them with an entry, + * and reopens it from a configuration which names {@code reopenedWith} instead - null for a + * configuration which names no index at all. The trees of everything it no longer names are left + * behind, and the entry added afterwards is in none of them. + */ + private LeftoverBackend leaveTreesBehind(StorageKind kind, BackendIndexCfg reopenedWith) throws Exception + { + final LeftoverBackend indexed = + openBackend(kind, true, indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING)), vlvIndexCfg()); + final Set indexTrees; + final TreeName vlvTree; + final TreeName vlvCounterTree; + try + { + addEntry(indexed, "dn: " + BASE_DN, "objectClass: top", "objectClass: domain", "dc: b990"); + addEntry(indexed, "dn: cn=stale," + BASE_DN, "objectClass: top", "objectClass: organizationalRole", + "cn: stale"); + final EntryContainer ec = indexed.getRootContainer().getEntryContainer(BASE_DN); + indexTrees = treesOf(ec.getAttributeIndex(cnType)); + vlvTree = new TreeName(ec.getTreePrefix(), "vlv." + VLV_INDEX_NAME); + vlvCounterTree = new TreeName(ec.getTreePrefix(), "counter.vlv." + VLV_INDEX_NAME); + } + finally + { + indexed.finalizeBackend(); + } + + final LeftoverBackend reopened = openBackend(kind, false, reopenedWith, null); + try + { + addEntry(reopened, "dn: cn=fresh," + BASE_DN, "objectClass: top", "objectClass: organizationalRole", + "cn: fresh"); + } + catch (Exception e) + { + reopened.finalizeBackend(); + throw e; + } + reopened.leftoverIndexTrees = indexTrees; + reopened.leftoverVlvTree = vlvTree; + reopened.leftoverVlvCounterTree = vlvCounterTree; + return reopened; + } + + /** The first key each of the given trees holds, for the trees which hold any. */ + private static Map keysHeldBy(Storage storage, Set trees) throws Exception + { + final Map keys = new HashMap<>(); + for (final TreeName tree : trees) + { + final ByteString key = storage.read(txn -> { + try (Cursor cursor = txn.openCursor(tree)) + { + return cursor.next() ? cursor.getKey() : null; + } + }); + if (key != null) + { + keys.put(tree, key); + } + } + return keys; + } + + private static boolean holdsAnything(Storage storage, TreeName tree) throws Exception + { + return storage.read(txn -> { + try (Cursor cursor = txn.openCursor(tree)) + { + return cursor.next(); + } + }); + } + + /** + * Whether the index answers the key with a non-empty entry ID set, as a search reading it would get one. + * {@code DefaultIndex.get} answers {@code newDefinedSet()} for a missing key while the index is trusted, so + * {@code isDefined()} alone would pin the trusted flag rather than the content a leftover tree still holds. + */ + private static boolean answers(Storage storage, MatchingRuleIndex index, ByteString key) throws Exception + { + return storage.read(txn -> { + final EntryIDSet ids = index.get(txn, key); + return ids.isDefined() && ids.size() > 0; + }); + } + + /** Reads back the flags an index tree carries, as they are stored. */ + private EnumSet persistedFlags(LeftoverBackend backend, TreeName index) throws Exception + { + final EntryContainer ec = backend.getRootContainer().getEntryContainer(BASE_DN); + final State state = new State(new TreeName(ec.getTreePrefix(), STATE_INDEX_NAME)); + return backend.getRootContainer().getStorage().read(txn -> state.getIndexFlags(txn, index)); + } + + /** The messages a change carries, as the operator reads them: identity and arguments both. */ + private static List renderedOf(Collection messages) + { + final List rendered = new ArrayList<>(); + for (final LocalizableMessage message : messages) + { + rendered.add(message.toString()); + } + return rendered; + } + + /** The messages a change carries, by identity rather than by their formatted text. */ + private static Set ordinalsOf(Collection messages) + { + final Set ordinals = new HashSet<>(); + for (final LocalizableMessage message : messages) + { + ordinals.add(message.ordinal()); + } + return ordinals; + } + + /** + * Registers a mock {@code ErrorLogPublisher}, enabled for every category and severity, so that + * {@link #warningOrdinalsLoggedTo} can read back what {@code AttributeIndex.reportDiscardedLeftovers} + * put in the error log - the half of it the change result alone does not exercise. The caller removes + * it once done, from a {@code finally}: it would otherwise go on capturing messages other tests log. + */ + @SuppressWarnings("unchecked") + private static ErrorLogPublisher registerErrorLogCapture() + { + final ErrorLogPublisher errorLog = mock(ErrorLogPublisher.class); + when(errorLog.isEnabledFor(any(), any())).thenReturn(true); + ErrorLogger.getInstance().addLogPublisher(errorLog); + return errorLog; + } + + /** The ordinals of the messages logged at {@link Severity#WARNING} through the given capture. */ + private static Set warningOrdinalsLoggedTo(ErrorLogPublisher errorLog) + { + final ArgumentCaptor logged = ArgumentCaptor.forClass(LocalizableMessage.class); + verify(errorLog, atLeastOnce()).log(any(), eq(Severity.WARNING), logged.capture(), any()); + return ordinalsOf(logged.getAllValues()); + } + + private static Set treesOf(AttributeIndex index) + { + final Set names = new HashSet<>(); + for (final MatchingRuleIndex matchingRuleIndex : index.getNameToIndexes().values()) + { + names.add(matchingRuleIndex.getName()); + } + return names; + } + + private static void addEntry(LeftoverBackend backend, String... ldifLines) throws Exception + { + final Entry entry = TestCaseUtils.makeEntry(ldifLines); + backend.addEntry(entry, mock(AddOperation.class)); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static ConfigurationAddListener indexAddListener(LeftoverBackend backend) + throws ConfigException + { + final ArgumentCaptor captor = ArgumentCaptor.forClass(ConfigurationAddListener.class); + verify(backend.configuredWith).addBackendIndexAddListener(captor.capture()); + return captor.getValue(); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private static ConfigurationAddListener vlvIndexAddListener(LeftoverBackend backend) + throws ConfigException + { + final ArgumentCaptor captor = ArgumentCaptor.forClass(ConfigurationAddListener.class); + verify(backend.configuredWith).addBackendVLVIndexAddListener(captor.capture()); + return captor.getValue(); + } + + /** + * Opens a backend whose configuration names the given index and VLV index - null for one it does + * not name, whose trees are then left to whatever is already in the storage. + */ + private LeftoverBackend openBackend(StorageKind kind, boolean pristine, BackendIndexCfg cnIndexCfg, + BackendVLVIndexCfg vlvIndexCfg) throws Exception + { + final LeftoverBackend backend = new LeftoverBackend(kind); + backend.setBackendID(BACKEND_ID); + backend.cnIndexCfg = cnIndexCfg != null ? cnIndexCfg + : indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.SUBSTRING)); + backend.vlvIndexCfg = vlvIndexCfg != null ? vlvIndexCfg : vlvIndexCfg(); + backend.configuredWith = backendCfg(kind, cnIndexCfg, vlvIndexCfg); + backend.configureBackend(backend.configuredWith, serverContext); + if (pristine) + { + // Start from a pristine on-disk state, so that a previous run cannot mask what this one leaves. + backend.storage.removeStorageFiles(); + } + try + { + backend.openBackend(); + } + catch (Exception e) + { + // openBackend() opens the root container before it registers the base DNs and the monitor, so a failure in + // any of those leaves the volume open and every following test failing here too. finalizeBackend() is no + // use for it: closeBackend() deregisters the monitor before it closes the root container, and NPEs on the + // monitor which was never registered. + try + { + final RootContainer root = backend.getRootContainer(); + if (root != null) + { + root.close(); + } + else + { + backend.storage.close(); + } + } + catch (Exception cleanupFailure) + { + e.addSuppressed(cleanupFailure); + } + throw e; + } + return backend; + } + + private PluggableBackendCfg backendCfg(StorageKind kind, BackendIndexCfg cnIndexCfg, BackendVLVIndexCfg vlvIndexCfg) + throws ConfigException + { + final PluggableBackendCfg cfg = kind.newBackendCfg(BACKEND_ID + "." + kind); + when(cfg.dn()).thenReturn(DN.valueOf("ds-cfg-backend-id=" + BACKEND_ID + ",cn=Backends,cn=config")); + when(cfg.getBackendId()).thenReturn(BACKEND_ID); + when(cfg.getBaseDN()).thenReturn(newTreeSet(BASE_DN)); + when(cfg.listBackendIndexes()).thenReturn(cnIndexCfg != null ? new String[] { "cn" } : new String[0]); + when(cfg.getBackendIndex("cn")).thenReturn(cnIndexCfg); + when(cfg.listBackendVLVIndexes()) + .thenReturn(vlvIndexCfg != null ? new String[] { VLV_INDEX_NAME } : new String[0]); + when(cfg.getBackendVLVIndex(VLV_INDEX_NAME)).thenReturn(vlvIndexCfg); + return cfg; + } + + private BackendIndexCfg indexCfg(SortedSet indexTypes) + { + final BackendIndexCfg cfg = mock(BackendIndexCfg.class); + when(cfg.dn()).thenReturn(DN.valueOf("ds-cfg-attribute=cn,cn=Index,ds-cfg-backend-id=" + BACKEND_ID + + ",cn=Backends,cn=config")); + when(cfg.getIndexType()).thenReturn(indexTypes); + when(cfg.getAttribute()).thenReturn(cnType); + when(cfg.getIndexEntryLimit()).thenReturn(4000); + when(cfg.getSubstringLength()).thenReturn(6); + return cfg; + } + + private BackendVLVIndexCfg vlvIndexCfg() + { + final BackendVLVIndexCfg cfg = mock(BackendVLVIndexCfg.class); + when(cfg.getName()).thenReturn(VLV_INDEX_NAME); + when(cfg.getBaseDN()).thenReturn(BASE_DN); + when(cfg.getScope()).thenReturn(Scope.WHOLE_SUBTREE); + when(cfg.getFilter()).thenReturn("(objectClass=*)"); + when(cfg.getSortOrder()).thenReturn("+cn"); + return cfg; + } + + /** A backend over the storage of the given kind, which keeps hold of the configuration its entry container registered with. */ + private static final class LeftoverBackend extends BackendImpl + { + private final StorageKind kind; + private RefusingStorage storage; + /** The configuration the entry container registers its listeners with. */ + private PluggableBackendCfg configuredWith; + private BackendIndexCfg cnIndexCfg; + private BackendVLVIndexCfg vlvIndexCfg; + /** The trees of the indexes an earlier configuration named, which nothing names now. */ + private Set leftoverIndexTrees; + private TreeName leftoverVlvTree; + private TreeName leftoverVlvCounterTree; + + private LeftoverBackend(StorageKind kind) + { + this.kind = kind; + } + + @Override + protected Storage configureStorage(PluggableBackendCfg cfg, ServerContext serverContext) throws ConfigException + { + storage = new RefusingStorage(kind.newStorage(cfg, serverContext)); + return storage; + } + } + + /** A failure no storage engine replays, unlike a conflict. */ + private static final class RefusedWrite extends Exception + { + private static final long serialVersionUID = 1L; + + RefusedWrite() + { + super("write refused by the test"); + } + } + + /** + * Delegates to the storage of the engine, and refuses the one write a case arms: neither engine + * can be made to fail a write on its own, and both are final. The refusal comes at one of two + * points - before the operation runs, which is what a write the engine cannot begin looks like + * to the caller, or once the operation has run and before it is committed, which is what a + * failure at commit looks like: the engine rolls the operation back, and whatever the operation + * noted on the way stands. Unarmed, it is the engine's storage. + */ + private static final class RefusingStorage implements Storage + { + private final Storage delegate; + /** Which write, counted over the life of this storage, is armed; zero for none. */ + private int armedWrite; + private boolean afterItRan; + private int writes; + + RefusingStorage(Storage delegate) + { + this.delegate = delegate; + } + + /** Refuses the {@code nth} write asked for from now on, the next one being the first, before it runs. */ + void refuseWrite(int nth) + { + armedWrite = writes + nth; + afterItRan = false; + } + + /** Fails the {@code nth} write asked for from now on once its operation has run, before it is committed. */ + void failWriteAfterItRan(int nth) + { + armedWrite = writes + nth; + afterItRan = true; + } + + /** How many write operations this storage was asked for, refused or not. */ + int writes() + { + return writes; + } + + @Override + public void write(final WriteOperation operation) throws Exception + { + writes++; + if (writes != armedWrite) + { + delegate.write(operation); + return; + } + armedWrite = 0; + if (!afterItRan) + { + throw new RefusedWrite(); + } + delegate.write(new WriteOperation() + { + @Override + public void run(WriteableTransaction txn) throws Exception + { + operation.run(txn); + throw new RefusedWrite(); + } + }); + } + + @Override + public Importer startImport() throws ConfigException + { + return delegate.startImport(); + } + + @Override + public void open(AccessMode accessMode) throws Exception + { + delegate.open(accessMode); + } + + @Override + public T read(ReadOperation readOperation) throws Exception + { + return delegate.read(readOperation); + } + + @Override + public void removeStorageFiles() + { + delegate.removeStorageFiles(); + } + + @Override + public StorageStatus getStorageStatus() + { + return delegate.getStorageStatus(); + } + + @Override + public boolean supportsBackupAndRestore() + { + return delegate.supportsBackupAndRestore(); + } + + @Override + public void createBackup(BackupConfig backupConfig) throws DirectoryException + { + delegate.createBackup(backupConfig); + } + + @Override + public void removeBackup(BackupDirectory backupDirectory, String backupID) throws DirectoryException + { + delegate.removeBackup(backupDirectory, backupID); + } + + @Override + public void restoreBackup(RestoreConfig restoreConfig) throws DirectoryException + { + delegate.restoreBackup(restoreConfig); + } + + @Override + public Set listTrees() + { + return delegate.listTrees(); + } + + @Override + public void close() + { + delegate.close(); + } + } +} diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java index 5c4897a8c2..cc030d8519 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ReplayedConfigChangeTest.java @@ -597,8 +597,9 @@ public void anIndexAddedByAChangeIsReportedOnceWhenTheTransactionIsReplayed() th 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); + // The second of the four writes a change makes is the one which opens the indexes it adds; + // the first drops what an earlier index left behind for them. + backend.storage.conflictAtCommitOnWrite(2, 1); final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY, IndexType.PRESENCE), 4000)); @@ -634,7 +635,8 @@ public void aRaisedEntryLimitUntrustsTheIndexWhenTheTransactionIsReplayed() thro 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. + // change adds and delete the ones it removes, and it neither adds nor removes any - so the + // write which drops what an earlier index left behind for the added ones is not made. final int writesBefore = backend.storage.writes(); backend.storage.conflictAtCommitOnWrite(3, 1); final ConfigChangeResult ccr = index.applyConfigurationChange(indexCfg(newTreeSet(IndexType.EQUALITY), 8000));