From f09c069e92d051036af2a969fe5289cb7c4826ba Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Mon, 26 Oct 2015 08:22:49 +0000
Subject: [PATCH] OPENDJ-2349: fix deadlocks during subtree deletes and moddn
---
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2CountTest.java | 29
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java | 45 ++
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/CursorTransformer.java | 13
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DefaultIndexTest.java | 6
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/SequentialCursor.java | 12
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java | 9
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Count.java | 45 +
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java | 8
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java | 24 +
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java | 834 ++++++++++++++++---------------------------
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/OnDiskMergeImporterTest.java | 6
opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java | 14
opendj-server-legacy/src/main/java/org/opends/server/backends/jeb/JEStorage.java | 23 +
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java | 49 ++
14 files changed, 566 insertions(+), 551 deletions(-)
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 69314d0..2ad53f2 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
@@ -101,6 +101,7 @@
import com.sleepycat.je.EnvironmentConfig;
import com.sleepycat.je.OperationStatus;
import com.sleepycat.je.Transaction;
+import com.sleepycat.je.TransactionConfig;
/** Berkeley DB Java Edition (JE for short) database implementation of the {@link Storage} engine. */
public final class JEStorage implements Storage, Backupable, ConfigurationChangeListener<JEBackendCfg>,
@@ -171,6 +172,20 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throwIfNotSuccess();
+ try
+ {
+ cursor.delete();
+ }
+ catch (DatabaseException e)
+ {
+ throw new StorageRuntimeException(e);
+ }
+ }
+
+ @Override
public boolean positionToKey(final ByteSequence key)
{
clearCurrentKeyAndValue();
@@ -568,6 +583,10 @@
}
private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+
+ /** Use read committed isolation instead of the default which is repeatable read. */
+ private static final TransactionConfig TXN_READ_COMMITTED = new TransactionConfig().setReadCommitted(true);
+
private final ServerContext serverContext;
private final File backendDirectory;
private JEBackendCfg config;
@@ -609,7 +628,7 @@
tree = trees.get(treeName);
if (tree == null)
{
- tree = env.openDatabase(txn, toDatabaseName(treeName), dbConfig());
+ tree = env.openDatabase(null, toDatabaseName(treeName), dbConfig());
trees.put(treeName, tree);
}
}
@@ -782,7 +801,7 @@
{
if (envConfig.getTransactional())
{
- final Transaction txn = env.beginTransaction(null, null);
+ final Transaction txn = env.beginTransaction(null, TXN_READ_COMMITTED);
logger.trace("beginTransaction txnid=%d", txn.getId());
return txn;
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
index 8367662..ca2dd15 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java
@@ -177,6 +177,20 @@
}
@Override
+ public void delete()
+ {
+ throwIfUndefined();
+ try
+ {
+ exchange.remove();
+ }
+ catch (final PersistitException e)
+ {
+ throw new StorageRuntimeException(e);
+ }
+ }
+
+ @Override
public boolean positionToKey(final ByteSequence key)
{
clearCurrentKeyAndValue();
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/CursorTransformer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/CursorTransformer.java
index c8423db..7bd3059 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/CursorTransformer.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/CursorTransformer.java
@@ -158,6 +158,13 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ // No need to clear the cached key/value. They will be updated when the cursor moves.
+ input.delete();
+ }
+
+ @Override
public boolean next()
{
clearCache();
@@ -238,6 +245,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ delegate.delete();
+ }
+
+ @Override
public void close()
{
delegate.close();
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
index 903b98f..1d0704d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DN2ID.java
@@ -29,7 +29,9 @@
import static org.opends.server.backends.pluggable.CursorTransformer.*;
import static org.opends.server.backends.pluggable.DnKeyFormat.*;
+import java.util.ArrayList;
import java.util.LinkedList;
+import java.util.List;
import java.util.NoSuchElementException;
import org.forgerock.opendj.ldap.ByteSequence;
@@ -37,6 +39,7 @@
import org.forgerock.opendj.ldap.ByteStringBuilder;
import org.forgerock.opendj.ldap.Functions;
import org.forgerock.util.Function;
+import org.forgerock.util.Pair;
import org.forgerock.util.promise.NeverThrowsException;
import org.opends.server.backends.pluggable.OnDiskMergeImporter.SequentialCursorDecorator;
import org.opends.server.backends.pluggable.spi.Cursor;
@@ -46,7 +49,9 @@
import org.opends.server.backends.pluggable.spi.TreeName;
import org.opends.server.backends.pluggable.spi.UpdateFunction;
import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.opends.server.types.CanceledOperationException;
import org.opends.server.types.DN;
+import org.opends.server.types.Operation;
/**
* This class represents the dn2id index, which has one record
@@ -178,6 +183,44 @@
return transformKeysAndValues(new SubtreeCursor(openCursor0(txn, dn)), TO_VOID_KEY, TO_ENTRY_ID);
}
+ List<Pair<Long, Long>> renameSubtree(WriteableTransaction txn,
+ DN oldName,
+ DN newName,
+ RootContainer rootContainer,
+ boolean renumberEntryIDs,
+ Operation operation)
+ throws CanceledOperationException
+ {
+ try (SequentialCursor<ByteString, ByteString> cursor = new SubtreeCursor(openCursor0(txn, oldName)))
+ {
+ List<Pair<Long, Long>> renamedEntryIDs = new ArrayList<>();
+ int oldTargetDnKeyLength = toKey(oldName).length();
+ ByteString newTargetDnKey = toKey(newName);
+
+ do
+ {
+ ByteString currentDnKey = cursor.getKey();
+ EntryID oldID = new EntryID(cursor.getValue());
+ cursor.delete();
+
+ ByteString newDnKeySuffix = currentDnKey.subSequence(oldTargetDnKeyLength, currentDnKey.length());
+ ByteSequence newDnKey = new ByteStringBuilder(newTargetDnKey).append(newDnKeySuffix);
+ EntryID newID = renumberEntryIDs ? rootContainer.getNextEntryID() : oldID;
+ txn.put(getName(), newDnKey, newID.toByteString());
+
+ renamedEntryIDs.add(Pair.of(oldID.longValue(), newID.longValue()));
+
+ if (operation != null)
+ {
+ operation.checkIfCanceled(false);
+ }
+ }
+ while (cursor.next());
+
+ return renamedEntryIDs;
+ }
+ }
+
/**
* Check if two DN have a parent-child relationship.
*
@@ -389,6 +432,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
delegate.close();
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 cdd499c..4b27c46 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
@@ -47,6 +47,7 @@
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
+import java.util.Objects;
import java.util.TreeMap;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -61,6 +62,7 @@
import org.forgerock.opendj.ldap.ByteStringBuilder;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SearchScope;
+import org.forgerock.util.Pair;
import org.opends.messages.CoreMessages;
import org.opends.server.admin.server.ConfigurationAddListener;
import org.opends.server.admin.server.ConfigurationChangeListener;
@@ -1464,7 +1466,30 @@
throw new DirectoryException(ResultCode.ENTRY_ALREADY_EXISTS,
ERR_ADD_ENTRY_ALREADY_EXISTS.get(entry.getName()));
}
- addEntry0(entry, parentDN, entryID, indexBuffer, encodedEntry, txn);
+ // Check that the parent entry exists.
+ EntryID parentID = null;
+ if (parentDN != null)
+ {
+ // Check for referral entries above the target.
+ dn2uri.targetEntryReferrals(txn, entry.getName(), null);
+
+ parentID = dn2id.get(txn, parentDN);
+ if (parentID == null)
+ {
+ throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
+ ERR_ADD_NO_SUCH_OBJECT.get(entry.getName()),
+ getMatchedDN(txn, parentDN),
+ null);
+ }
+ }
+
+ // Ensure same access ordering as deleteEntry.
+ dn2id.put(txn, entry.getName(), entryID);
+ id2childrenCount.updateCount(txn, parentID, 1);
+ id2entry.put(txn, entryID, encodedEntry);
+ dn2uri.addEntry(txn, entry);
+ id2childrenCount.updateTotalCount(txn, 1);
+ indexBuffer.flush(txn);
if (addOperation != null)
{
// One last check before committing
@@ -1500,37 +1525,15 @@
}
}
- private void addEntry0(final Entry entry, final DN parentDN, final EntryID entryID, final IndexBuffer indexBuffer,
- final ByteString encodedEntry, WriteableTransaction txn) throws DirectoryException
- {
- // Check that the parent entry exists.
- if (parentDN != null)
- {
- // Check for referral entries above the target.
- dn2uri.targetEntryReferrals(txn, entry.getName(), null);
-
- final EntryID parentID = dn2id.get(txn, parentDN);
- if (parentID == null)
- {
- throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
- ERR_ADD_NO_SUCH_OBJECT.get(entry.getName()), getMatchedDN(txn, baseDN), null);
- }
- id2childrenCount.addDelta(txn, parentID, 1);
- }
-
- dn2id.put(txn, entry.getName(), entryID);
- dn2uri.addEntry(txn, entry);
- id2entry.put(txn, entryID, encodedEntry);
-
- indexBuffer.flush(txn);
- }
-
void importEntry(WriteableTransaction txn, EntryID entryID, Entry entry) throws DirectoryException,
StorageRuntimeException
{
final IndexBuffer indexBuffer = IndexBuffer.newImportIndexBuffer(txn, entryID);
insertEntryIntoIndexes(indexBuffer, entry, entryID);
- addEntry0(entry, null, entryID, indexBuffer, id2entry.encode(entry), txn);
+ dn2id.put(txn, entry.getName(), entryID);
+ id2entry.put(txn, entryID, id2entry.encode(entry));
+ dn2uri.addEntry(txn, entry);
+ indexBuffer.flush(txn);
}
/**
@@ -1550,22 +1553,8 @@
* @throws CanceledOperationException if this operation should be cancelled.
*/
void deleteEntry(final DN entryDN, final DeleteOperation deleteOperation)
- throws DirectoryException, StorageRuntimeException, CanceledOperationException
+ throws DirectoryException, StorageRuntimeException, CanceledOperationException
{
- final IndexBuffer indexBuffer = new IndexBuffer();
- final boolean isSubtreeDelete =
- deleteOperation != null && deleteOperation.getRequestControl(SubtreeDeleteControl.DECODER) != null;
-
- /*
- * We will iterate forwards through a range of the dn2id keys to find subordinates of the target entry from the top
- * of the tree downwards.
- */
- final ByteString entryDNKey = dnToDNKey(entryDN, baseDN.size());
- final ByteStringBuilder suffix = beforeKey(entryDNKey);
- final ByteStringBuilder end = afterKey(entryDNKey);
-
- final DN parentDN = getParentWithinBase(entryDN);
-
try
{
storage.write(new WriteOperation()
@@ -1578,90 +1567,109 @@
// Check for referral entries above the target entry.
dn2uri.targetEntryReferrals(txn, entryDN, null);
- int subordinateEntriesDeleted = 0;
-
- // Since everything under targetDN will be deleted, we only have to decrement the counter of targetDN's
- // parent. Other counters will be removed in deleteEntry()
- if (parentDN != null) {
- final EntryID parentID = dn2id.get(txn, parentDN);
- if ( parentID == null ) {
- throw new StorageRuntimeException(ERR_MISSING_DN2ID_RECORD.get(parentDN).toString());
+ // We'll need the parent ID when we update the id2childrenCount. Fetch it now so that accesses to dn2id
+ // are ordered.
+ final DN parentDN = getParentWithinBase(entryDN);
+ EntryID parentID = null;
+ if (parentDN != null)
+ {
+ parentID = dn2id.get(txn, parentDN);
+ if (parentID == null)
+ {
+ throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
+ ERR_DELETE_NO_SUCH_OBJECT.get(entryDN),
+ getMatchedDN(txn, parentDN),
+ null);
}
- id2childrenCount.addDelta(txn, parentID, -1);
}
- Cursor<ByteString, ByteString> cursor = txn.openCursor(dn2id.getName());
- try
+ // Delete the subordinate entries in dn2id if requested.
+ final boolean isSubtreeDelete = deleteOperation != null
+ && deleteOperation.getRequestControl(SubtreeDeleteControl.DECODER) != null;
+
+ /* draft-armijo-ldap-treedelete, 4.1 Tree Delete Semantics: The server MUST NOT chase referrals stored in
+ * the tree. If information about referrals is stored in this section of the tree, this pointer will be
+ * deleted.
+ */
+ final boolean isManageDsaIT = isSubtreeDelete || isManageDsaITOperation(deleteOperation);
+
+ /* Ensure that all index updates are done in the correct order to avoid deadlocks. First iterate over
+ * dn2id collecting all the IDs of the entries to be deleted. Then update dn2uri, id2entry,
+ * id2childrenCount, and finally the attribute indexes.
+ */
+ final List<Long> entriesToBeDeleted = new ArrayList<>();
+ try (final SequentialCursor<Void, EntryID> cursor = dn2id.openSubordinatesCursor(txn, entryDN))
{
- // Step forward until we pass the ending value.
- boolean success = cursor.positionToKeyOrNext(suffix);
- while (success && cursor.getKey().compareTo(end) < 0)
+ // Delete the target entry in dn2id.
+ if (!cursor.isDefined())
{
- // We have found a subordinate entry.
+ throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
+ ERR_DELETE_NO_SUCH_OBJECT.get(entryDN),
+ getMatchedDN(txn, entryDN),
+ null);
+ }
+ entriesToBeDeleted.add(cursor.getValue().longValue());
+ cursor.delete();
+
+ // Now delete the subordinate entries in dn2id.
+ while (cursor.next())
+ {
if (!isSubtreeDelete)
{
- // The subtree delete control was not specified and
- // the target entry is not a leaf.
- throw new DirectoryException(ResultCode.NOT_ALLOWED_ON_NONLEAF, ERR_DELETE_NOT_ALLOWED_ON_NONLEAF
- .get(entryDN));
+ throw new DirectoryException(ResultCode.NOT_ALLOWED_ON_NONLEAF,
+ ERR_DELETE_NOT_ALLOWED_ON_NONLEAF.get(entryDN));
}
-
- /*
- * Delete this entry which by now must be a leaf because we have
- * been deleting from the bottom of the tree upwards.
- */
- EntryID entryID = new EntryID(cursor.getValue());
-
- // Invoke any subordinate delete plugins on the entry.
- if (deleteOperation != null && !deleteOperation.isSynchronizationOperation())
- {
- Entry subordinateEntry = id2entry.get(txn, entryID);
- SubordinateDelete pluginResult =
- getPluginConfigManager().invokeSubordinateDeletePlugins(deleteOperation, subordinateEntry);
-
- if (!pluginResult.continueProcessing())
- {
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
- ERR_DELETE_ABORTED_BY_SUBORDINATE_PLUGIN.get(subordinateEntry.getName()));
- }
- }
-
- deleteEntry(txn, indexBuffer, true, entryDN, cursor.getKey(), entryID);
- subordinateEntriesDeleted++;
-
- if (deleteOperation != null)
- {
- deleteOperation.checkIfCanceled(false);
- }
-
- // Get the next DN.
- success = cursor.next();
+ entriesToBeDeleted.add(cursor.getValue().longValue());
+ cursor.delete();
+ checkIfCanceled(false);
}
}
- finally
+ // The target entry will have the lowest entryID so it will remain the first element.
+ Collections.sort(entriesToBeDeleted);
+
+ // Now update id2entry, dn2uri, and id2childrenCount in key order.
+ id2childrenCount.updateCount(txn, parentID, -1);
+ final IndexBuffer indexBuffer = new IndexBuffer();
+ final EntryCache<?> entryCache = DirectoryServer.getEntryCache();
+ boolean isBaseEntry = true;
+ try (final Cursor<EntryID, Entry> cursor = id2entry.openCursor(txn))
{
- cursor.close();
+ for (Long entryIDLong : entriesToBeDeleted)
+ {
+ final EntryID entryID = new EntryID(entryIDLong);
+ if (!cursor.positionToKey(entryID.toByteString()))
+ {
+ throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
+ ERR_MISSING_ID2ENTRY_RECORD.get(entryID));
+ }
+ final Entry entry = cursor.getValue();
+ if (isBaseEntry && !isManageDsaIT)
+ {
+ dn2uri.checkTargetForReferral(entry, null);
+ }
+ cursor.delete();
+ dn2uri.deleteEntry(txn, entry);
+ id2childrenCount.removeCount(txn, entryID);
+ removeEntryFromIndexes(indexBuffer, entry, entryID);
+ if (!isBaseEntry)
+ {
+ invokeSubordinateDeletePlugins(entry);
+ }
+ if (entryCache != null)
+ {
+ entryCache.removeEntry(entry.getName());
+ }
+ isBaseEntry = false;
+ checkIfCanceled(false);
+ }
}
-
- // draft-armijo-ldap-treedelete, 4.1 Tree Delete Semantics:
- // The server MUST NOT chase referrals stored in the tree. If
- // information about referrals is stored in this section of the
- // tree, this pointer will be deleted.
- boolean manageDsaIT = isSubtreeDelete || isManageDsaITOperation(deleteOperation);
- deleteEntry(txn, indexBuffer, manageDsaIT, entryDN, null, null);
-
+ id2childrenCount.updateTotalCount(txn, -entriesToBeDeleted.size());
indexBuffer.flush(txn);
-
- if (deleteOperation != null)
- {
- // One last check before committing
- deleteOperation.checkIfCanceled(true);
- }
-
+ checkIfCanceled(true);
if (isSubtreeDelete)
{
deleteOperation.addAdditionalLogItem(unquotedKeyValue(getClass(), "deletedEntries",
- subordinateEntriesDeleted + 1));
+ entriesToBeDeleted.size()));
}
}
catch (StorageRuntimeException | DirectoryException | CanceledOperationException e)
@@ -1679,6 +1687,28 @@
DirectoryServer.getServerErrorResultCode(), ERR_UNCHECKED_EXCEPTION.get(msg), e);
}
}
+
+ private void invokeSubordinateDeletePlugins(final Entry entry) throws DirectoryException
+ {
+ if (deleteOperation != null && !deleteOperation.isSynchronizationOperation())
+ {
+ SubordinateDelete pluginResult =
+ getPluginConfigManager().invokeSubordinateDeletePlugins(deleteOperation, entry);
+ if (!pluginResult.continueProcessing())
+ {
+ throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
+ ERR_DELETE_ABORTED_BY_SUBORDINATE_PLUGIN.get(entry.getName()));
+ }
+ }
+ }
+
+ private void checkIfCanceled(boolean signalTooLate) throws CanceledOperationException
+ {
+ if (deleteOperation != null)
+ {
+ deleteOperation.checkIfCanceled(signalTooLate);
+ }
+ }
});
}
catch (Exception e)
@@ -1687,78 +1717,6 @@
}
}
- private void deleteEntry(WriteableTransaction txn,
- IndexBuffer indexBuffer,
- boolean manageDsaIT,
- DN targetDN,
- ByteSequence leafDNKey,
- EntryID leafID)
- throws StorageRuntimeException, DirectoryException
- {
- if(leafID == null || leafDNKey == null)
- {
- // Read the entry ID from dn2id.
- if(leafDNKey == null)
- {
- leafDNKey = dnToDNKey(targetDN, baseDN.size());
- }
- // FIXME: previously this used a RMW lock - see OPENDJ-1878.
- ByteString value = txn.read(dn2id.getName(), leafDNKey);
- if (value == null)
- {
- LocalizableMessage message = ERR_DELETE_NO_SUCH_OBJECT.get(targetDN);
- DN matchedDN = getMatchedDN(txn, baseDN);
- throw new DirectoryException(ResultCode.NO_SUCH_OBJECT, message, matchedDN, null);
- }
- leafID = new EntryID(value);
- }
-
- // Remove from dn2id.
- if (!txn.delete(dn2id.getName(), leafDNKey))
- {
- // Do not expect to ever come through here.
- throw new DirectoryException(
- ResultCode.NO_SUCH_OBJECT, ERR_DELETE_NO_SUCH_OBJECT.get(leafDNKey), getMatchedDN(txn, baseDN), null);
- }
-
- // Check that the entry exists in id2entry and read its contents.
- // FIXME: previously this used a RMW lock - see OPENDJ-1878.
- Entry entry = id2entry.get(txn, leafID);
- if (entry == null)
- {
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
- ERR_MISSING_ID2ENTRY_RECORD.get(leafID));
- }
-
- if (!manageDsaIT)
- {
- dn2uri.checkTargetForReferral(entry, null);
- }
-
- // Update the referral tree.
- dn2uri.deleteEntry(txn, entry);
-
- // Remove from id2entry.
- if (!id2entry.remove(txn, leafID))
- {
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
- ERR_MISSING_ID2ENTRY_RECORD.get(leafID));
- }
-
- // Remove from the indexes, in index config order.
- removeEntryFromIndexes(indexBuffer, entry, leafID);
-
- // Remove the children counter for this entry.
- id2childrenCount.deleteCount(txn, leafID);
-
- // Remove the entry from the entry cache.
- EntryCache<?> entryCache = DirectoryServer.getEntryCache();
- if (entryCache != null)
- {
- entryCache.removeEntry(entry.getName());
- }
- }
-
/**
* Indicates whether an entry with the specified DN exists.
*
@@ -1899,7 +1857,9 @@
if (entryID == null)
{
throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
- ERR_MODIFY_NO_SUCH_OBJECT.get(newEntry.getName()), getMatchedDN(txn, baseDN), null);
+ ERR_MODIFY_NO_SUCH_OBJECT.get(newEntry.getName()),
+ getMatchedDN(txn, newEntry.getName()),
+ null);
}
if (!isManageDsaITOperation(modifyOperation))
@@ -1908,6 +1868,9 @@
dn2uri.checkTargetForReferral(oldEntry, null);
}
+ // Ensure same ordering as deleteEntry: id2entry, dn2uri, then indexes.
+ id2entry.put(txn, entryID, newEntry);
+
// Update the referral tree.
if (modifyOperation != null)
{
@@ -1920,8 +1883,6 @@
dn2uri.replaceEntry(txn, oldEntry, newEntry);
}
- // Replace id2entry.
- id2entry.put(txn, entryID, newEntry);
// Update the indexes.
final IndexBuffer indexBuffer = new IndexBuffer();
@@ -1983,8 +1944,8 @@
* target DN of the provided entry. The caller must hold write locks on both
* the current DN and the new DN for the entry.
*
- * @param currentDN The current DN of the entry to be replaced.
- * @param entry The new content to use for the entry.
+ * @param oldTargetDN The current DN of the entry to be renamed.
+ * @param newTargetEntry The new content to use for the entry.
* @param modifyDNOperation The modify DN operation with which this action
* is associated. This may be <CODE>null</CODE>
* for modify DN operations performed internally.
@@ -1996,10 +1957,9 @@
* modify DN operation.
* @throws StorageRuntimeException If an error occurs in the storage.
*/
- void renameEntry(final DN currentDN, final Entry entry, final ModifyDNOperation modifyDNOperation)
+ void renameEntry(final DN oldTargetDN, final Entry newTargetEntry, final ModifyDNOperation modifyDNOperation)
throws StorageRuntimeException, DirectoryException, CanceledOperationException
{
- // FIXME: consistency + isolation cannot be maintained lock free - see OPENDJ-1878.
try
{
storage.write(new WriteOperation()
@@ -2007,168 +1967,80 @@
@Override
public void run(WriteableTransaction txn) throws Exception
{
- DN oldSuperiorDN = getParentWithinBase(currentDN);
- DN newSuperiorDN = getParentWithinBase(entry.getName());
-
- final boolean isApexEntryMoved;
- if (oldSuperiorDN != null)
- {
- isApexEntryMoved = !oldSuperiorDN.equals(newSuperiorDN);
- }
- else if (newSuperiorDN != null)
- {
- isApexEntryMoved = !newSuperiorDN.equals(oldSuperiorDN);
- }
- else
- {
- isApexEntryMoved = false;
- }
-
- final IndexBuffer buffer = new IndexBuffer();
-
try
{
- // Check whether the renamed entry already exists.
- if (!currentDN.equals(entry.getName()) && dn2id.get(txn, entry.getName()) != null)
- {
- LocalizableMessage message = ERR_MODIFYDN_ALREADY_EXISTS.get(entry.getName());
- throw new DirectoryException(ResultCode.ENTRY_ALREADY_EXISTS, message);
- }
+ // Validate the request.
+ final DN newTargetDN = newTargetEntry.getName();
+ final DN oldSuperiorDN = getParentWithinBase(oldTargetDN);
+ final DN newSuperiorDN = getParentWithinBase(newTargetDN);
- EntryID oldApexID = dn2id.get(txn, currentDN);
- if (oldApexID == null)
+ final EntryID oldSuperiorID = oldSuperiorDN != null ? dn2id.get(txn, oldSuperiorDN) : null;
+ final EntryID oldTargetID = dn2id.get(txn, oldTargetDN);
+ if ((oldSuperiorDN != null && oldSuperiorID == null) || oldTargetID == null)
{
// Check for referral entries above the target entry.
- dn2uri.targetEntryReferrals(txn, currentDN, null);
-
+ dn2uri.targetEntryReferrals(txn, oldTargetDN, null);
throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
- ERR_MODIFYDN_NO_SUCH_OBJECT.get(currentDN), getMatchedDN(txn, baseDN), null);
+ ERR_MODIFYDN_NO_SUCH_OBJECT.get(oldTargetDN),
+ getMatchedDN(txn, oldTargetDN),
+ null);
}
- Entry oldApexEntry = id2entry.get(txn, oldApexID);
- if (oldApexEntry == null)
+ final EntryID newSuperiorID = newSuperiorDN != null ? dn2id.get(txn, newSuperiorDN) : null;
+ if (newSuperiorDN != null && newSuperiorID == null)
{
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(), ERR_MISSING_ID2ENTRY_RECORD
- .get(oldApexID));
+ throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
+ ERR_NEW_SUPERIOR_NO_SUCH_OBJECT.get(newSuperiorDN),
+ getMatchedDN(txn, newSuperiorDN),
+ null);
}
-
- if (!isManageDsaITOperation(modifyDNOperation))
+ if (dn2id.get(txn, newTargetDN) != null)
{
- dn2uri.checkTargetForReferral(oldApexEntry, null);
+ throw new DirectoryException(ResultCode.ENTRY_ALREADY_EXISTS,
+ ERR_MODIFYDN_ALREADY_EXISTS.get(newTargetDN));
}
- EntryID newApexID = oldApexID;
- if (newSuperiorDN != null && isApexEntryMoved)
- {
- /*
- * We want to preserve the invariant that the ID of an entry is
- * greater than its parent, since search results are returned in
- * ID order.
- */
- EntryID newSuperiorID = dn2id.get(txn, newSuperiorDN);
- if (newSuperiorID == null)
- {
- throw new DirectoryException(ResultCode.NO_SUCH_OBJECT,
- ERR_NEW_SUPERIOR_NO_SUCH_OBJECT.get(newSuperiorDN), getMatchedDN(txn, baseDN), null);
- }
-
- if (newSuperiorID.compareTo(oldApexID) > 0)
- {
- // This move would break the above invariant so we must
- // renumber every entry that moves. This is even more
- // expensive since every entry has to be deleted from
- // and added back into the attribute indexes.
- newApexID = rootContainer.getNextEntryID();
-
- if (logger.isTraceEnabled())
- {
- logger.trace("Move of target entry requires renumbering" + "all entries in the subtree. "
- + "Old DN: %s " + "New DN: %s " + "Old entry ID: %d " + "New entry ID: %d "
- + "New Superior ID: %d" + oldApexEntry.getName(), entry.getName(), oldApexID,
- newApexID, newSuperiorID);
- }
- }
- }
-
- MovedEntry head = new MovedEntry(null, null, false);
- MovedEntry current = head;
- // Move or rename the apex entry.
- removeApexEntry(txn, buffer, oldSuperiorDN, oldApexID, newApexID, oldApexEntry, entry, isApexEntryMoved,
- modifyDNOperation, current);
- current = current.next;
-
- /*
- * We will iterate forwards through a range of the dn2id keys to
- * find subordinates of the target entry from the top of the tree
- * downwards.
+ /* We want to preserve the invariant that the ID of an entry is greater than its parent, since search
+ * results are returned in ID order. Note: if the superior has changed then oldSuperiorDN and
+ * newSuperiorDN will be non-null.
*/
- ByteString currentDNKey = dnToDNKey(currentDN, baseDN.size());
- ByteStringBuilder suffix = beforeKey(currentDNKey);
- ByteStringBuilder end = afterKey(currentDNKey);
+ final boolean superiorHasChanged = !Objects.equals(oldSuperiorDN, newSuperiorDN);
+ final boolean renumberEntryIDs = superiorHasChanged && newSuperiorID.compareTo(oldSuperiorID) > 0;
- Cursor<ByteString, ByteString> cursor = txn.openCursor(dn2id.getName());
- try
+ /* Ensure that all index updates are done in the correct order to avoid deadlocks. First iterate over
+ * dn2id collecting all the IDs of the entries to be renamed. Then update dn2uri, id2entry,
+ * id2childrenCount, and finally the attribute indexes.
+ */
+ final List<Pair<Long, Long>> renamedEntryIDs = dn2id.renameSubtree(txn,
+ oldTargetDN,
+ newTargetDN,
+ rootContainer,
+ renumberEntryIDs,
+ modifyDNOperation);
+
+ // The target entry will have the lowest entryID so it will remain the first element.
+ Collections.sort(renamedEntryIDs, Pair.<Long, Long>getPairComparator());
+
+ // Now update id2entry, dn2uri, and id2childrenCount in key order.
+ if (superiorHasChanged)
{
- // Step forward until we pass the ending value.
- boolean success = cursor.positionToKeyOrNext(suffix);
- while (success && cursor.getKey().compareTo(end) < 0)
+ id2childrenCount.updateCount(txn, oldSuperiorID, -1);
+ id2childrenCount.updateCount(txn, newSuperiorID, 1);
+ }
+ final IndexBuffer indexBuffer = new IndexBuffer();
+ boolean isBaseEntry = true;
+ try (final Cursor<EntryID, Entry> cursor = id2entry.openCursor(txn))
+ {
+ for (Pair<Long, Long> renamedEntryID : renamedEntryIDs)
{
- // We have found a subordinate entry.
- EntryID oldID = new EntryID(cursor.getValue());
- Entry oldEntry = id2entry.get(txn, oldID);
-
- // Construct the new DN of the entry.
- DN newDN = modDN(oldEntry.getName(), currentDN.size(), entry.getName());
-
- // Assign a new entry ID if we are renumbering.
- EntryID newID = oldID;
- if (!newApexID.equals(oldApexID))
- {
- newID = rootContainer.getNextEntryID();
-
- if (logger.isTraceEnabled())
- {
- logger.trace("Move of subordinate entry requires renumbering. "
- + "Old DN: %s New DN: %s Old entry ID: %d New entry ID: %d",
- oldEntry.getName(), newDN, oldID, newID);
- }
- }
-
- // Move this entry.
- removeSubordinateEntry(txn, buffer, oldID, newID, oldEntry, newDN, modifyDNOperation, current);
- current = current.next;
-
- if (modifyDNOperation != null)
- {
- modifyDNOperation.checkIfCanceled(false);
- }
-
- // Get the next DN.
- success = cursor.next();
+ renameSingleEntry(txn, renamedEntryID, cursor, indexBuffer, newTargetDN, renumberEntryIDs, isBaseEntry);
+ isBaseEntry = false;
+ checkIfCanceled(false);
}
- }
- finally
- {
- cursor.close();
- }
- // Set current to the first moved entry and null out the head.
- // This will allow processed moved entries to be GCed.
- current = head.next;
- head = null;
- while (current != null)
- {
- addRenamedEntry(txn, buffer, current.entryID, current.entry, isApexEntryMoved, current.renumbered,
- modifyDNOperation);
- current = current.next;
}
- buffer.flush(txn);
-
- if (modifyDNOperation != null)
- {
- // One last check before committing
- modifyDNOperation.checkIfCanceled(true);
- }
+ indexBuffer.flush(txn);
+ checkIfCanceled(true);
}
catch (StorageRuntimeException | DirectoryException | CanceledOperationException e)
{
@@ -2185,6 +2057,123 @@
DirectoryServer.getServerErrorResultCode(), ERR_UNCHECKED_EXCEPTION.get(msg), e);
}
}
+
+ private void renameSingleEntry(
+ final WriteableTransaction txn,
+ final Pair<Long, Long> renamedEntryID,
+ final Cursor<EntryID, Entry> cursor,
+ final IndexBuffer indexBuffer,
+ final DN newTargetDN,
+ final boolean renumberEntryIDs,
+ final boolean isBaseEntry) throws DirectoryException
+ {
+ final EntryID oldEntryID = new EntryID(renamedEntryID.getFirst());
+ final EntryID newEntryID = new EntryID(renamedEntryID.getSecond());
+ if (!cursor.positionToKey(oldEntryID.toByteString()))
+ {
+ throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
+ ERR_MISSING_ID2ENTRY_RECORD.get(oldEntryID));
+ }
+
+ final Entry oldEntry = cursor.getValue();
+ final Entry newEntry;
+ final List<Modification> modifications;
+ if (isBaseEntry)
+ {
+ if (!isManageDsaITOperation(modifyDNOperation))
+ {
+ dn2uri.checkTargetForReferral(oldEntry, null);
+ }
+ newEntry = newTargetEntry;
+ modifications = modifyDNOperation != null ? modifyDNOperation.getModifications() : null;
+ }
+ else
+ {
+ final DN newDN = modDN(oldEntry.getName(), oldTargetDN.size(), newTargetDN);
+ newEntry = oldEntry.duplicate(false);
+ newEntry.setDN(newDN);
+ modifications = invokeSubordinateModifyDNPlugins(oldEntry, newEntry);
+ }
+
+ if (renumberEntryIDs)
+ {
+ cursor.delete();
+ }
+ id2entry.put(txn, newEntryID, newEntry);
+ dn2uri.deleteEntry(txn, oldEntry);
+ dn2uri.addEntry(txn, newEntry);
+ if (renumberEntryIDs)
+ {
+ // In-order: new entryID is guaranteed to be greater than old entryID.
+ final long count = id2childrenCount.removeCount(txn, oldEntryID);
+ id2childrenCount.updateCount(txn, newEntryID, count);
+ }
+
+ if (renumberEntryIDs || modifications == null)
+ {
+ // Slow path: the entry has been renumbered so we need to fully re-index.
+ removeEntryFromIndexes(indexBuffer, oldEntry, oldEntryID);
+ insertEntryIntoIndexes(indexBuffer, newEntry, newEntryID);
+ }
+ else if (!modifications.isEmpty())
+ {
+ // Fast-path: the entryID has not changed so we only need to re-index the mods.
+ indexModifications(indexBuffer, oldEntry, newEntry, oldEntryID, modifications);
+ }
+
+ final EntryCache<?> entryCache = DirectoryServer.getEntryCache();
+ if (entryCache != null)
+ {
+ entryCache.removeEntry(oldEntry.getName());
+ }
+ }
+
+ private void checkIfCanceled(boolean signalTooLate) throws CanceledOperationException
+ {
+ if (modifyDNOperation != null)
+ {
+ modifyDNOperation.checkIfCanceled(signalTooLate);
+ }
+ }
+
+ private List<Modification> invokeSubordinateModifyDNPlugins(
+ final Entry oldEntry, final Entry newEntry) throws DirectoryException
+ {
+ final List<Modification> modifications = Collections.unmodifiableList(new ArrayList<Modification>(0));
+
+ // Create a new entry that is a copy of the old entry but with the new DN.
+ // Also invoke any subordinate modify DN plugins on the entry.
+ // FIXME -- At the present time, we don't support subordinate modify DN
+ // plugins that make changes to subordinate entries and therefore
+ // provide an unmodifiable list for the modifications element.
+ // FIXME -- This will need to be updated appropriately if we decided that
+ // these plugins should be invoked for synchronization operations.
+ if (modifyDNOperation != null && !modifyDNOperation.isSynchronizationOperation())
+ {
+ SubordinateModifyDN pluginResult = getPluginConfigManager().invokeSubordinateModifyDNPlugins(
+ modifyDNOperation, oldEntry, newEntry, modifications);
+
+ if (!pluginResult.continueProcessing())
+ {
+ throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
+ ERR_MODIFYDN_ABORTED_BY_SUBORDINATE_PLUGIN.get(oldEntry.getName(),
+ newEntry.getName()));
+ }
+
+ if (!modifications.isEmpty())
+ {
+ LocalizableMessageBuilder invalidReason = new LocalizableMessageBuilder();
+ if (!newEntry.conformsToSchema(null, false, false, false, invalidReason))
+ {
+ throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
+ ERR_MODIFYDN_ABORTED_BY_SUBORDINATE_SCHEMA_ERROR.get(oldEntry.getName(),
+ newEntry.getName(),
+ invalidReason));
+ }
+ }
+ }
+ return modifications;
+ }
});
}
catch (Exception e)
@@ -2193,186 +2182,6 @@
}
}
- /** Represents an renamed entry that was deleted from but yet to be added back. */
- private static final class MovedEntry
- {
- private EntryID entryID;
- private Entry entry;
- private MovedEntry next;
- private boolean renumbered;
-
- private MovedEntry(EntryID entryID, Entry entry, boolean renumbered)
- {
- this.entryID = entryID;
- this.entry = entry;
- this.renumbered = renumbered;
- }
- }
-
- private void addRenamedEntry(WriteableTransaction txn, IndexBuffer buffer,
- EntryID newID,
- Entry newEntry,
- boolean isApexEntryMoved,
- boolean renumbered,
- ModifyDNOperation modifyDNOperation)
- throws DirectoryException, StorageRuntimeException
- {
- // FIXME: the core server should validate that the new subtree location is empty.
- dn2id.put(txn, newEntry.getName(), newID);
- id2entry.put(txn, newID, newEntry);
- dn2uri.addEntry(txn, newEntry);
-
- if (renumbered || modifyDNOperation == null)
- {
- // Reindex the entry with the new ID.
- insertEntryIntoIndexes(buffer, newEntry, newID);
- }
-
- if(isApexEntryMoved)
- {
- final DN parentDN = getParentWithinBase(newEntry.getName());
- if (parentDN != null)
- {
- id2childrenCount.addDelta(txn, dn2id.get(txn, parentDN), 1);
- }
- }
- }
-
- private void removeApexEntry(WriteableTransaction txn, IndexBuffer buffer,
- DN oldSuperiorDN,
- EntryID oldID, EntryID newID,
- Entry oldEntry, Entry newEntry,
- boolean isApexEntryMoved,
- ModifyDNOperation modifyDNOperation,
- MovedEntry tail)
- throws DirectoryException, StorageRuntimeException
- {
- DN oldDN = oldEntry.getName();
-
- // Remove the old DN from dn2id.
- dn2id.remove(txn, oldDN);
-
- // Remove old ID from id2entry and put the new entry
- // (old entry with new DN) in id2entry.
- if (!newID.equals(oldID))
- {
- id2entry.remove(txn, oldID);
- }
-
- // Update any referral records.
- dn2uri.deleteEntry(txn, oldEntry);
-
- tail.next = new MovedEntry(newID, newEntry, !newID.equals(oldID));
-
- if(oldSuperiorDN != null && isApexEntryMoved)
- {
- // Since entry has moved, oldSuperiorDN has lost a child
- id2childrenCount.addDelta(txn, dn2id.get(txn, oldSuperiorDN), -1);
- }
-
- if (!newID.equals(oldID))
- {
- id2childrenCount.addDelta(txn, newID, id2childrenCount.deleteCount(txn, oldID));
- }
-
- if (!newID.equals(oldID) || modifyDNOperation == null)
- {
- // Reindex the entry with the new ID.
- removeEntryFromIndexes(buffer, oldEntry, oldID);
- }
- else
- {
- // Update the indexes if needed.
- indexModifications(buffer, oldEntry, newEntry, oldID,
- modifyDNOperation.getModifications());
- }
-
- // Remove the entry from the entry cache.
- EntryCache<?> entryCache = DirectoryServer.getEntryCache();
- if (entryCache != null)
- {
- entryCache.removeEntry(oldDN);
- }
- }
-
- private void removeSubordinateEntry(WriteableTransaction txn, IndexBuffer buffer,
- EntryID oldID, EntryID newID,
- Entry oldEntry, DN newDN,
- ModifyDNOperation modifyDNOperation,
- MovedEntry tail)
- throws DirectoryException, StorageRuntimeException
- {
- DN oldDN = oldEntry.getName();
- Entry newEntry = oldEntry.duplicate(false);
- newEntry.setDN(newDN);
- List<Modification> modifications =
- Collections.unmodifiableList(new ArrayList<Modification>(0));
-
- // Create a new entry that is a copy of the old entry but with the new DN.
- // Also invoke any subordinate modify DN plugins on the entry.
- // FIXME -- At the present time, we don't support subordinate modify DN
- // plugins that make changes to subordinate entries and therefore
- // provide an unmodifiable list for the modifications element.
- // FIXME -- This will need to be updated appropriately if we decided that
- // these plugins should be invoked for synchronization operations.
- if (modifyDNOperation != null && !modifyDNOperation.isSynchronizationOperation())
- {
- SubordinateModifyDN pluginResult =
- getPluginConfigManager().invokeSubordinateModifyDNPlugins(
- modifyDNOperation, oldEntry, newEntry, modifications);
-
- if (!pluginResult.continueProcessing())
- {
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
- ERR_MODIFYDN_ABORTED_BY_SUBORDINATE_PLUGIN.get(oldDN, newDN));
- }
-
- if (! modifications.isEmpty())
- {
- LocalizableMessageBuilder invalidReason = new LocalizableMessageBuilder();
- if (! newEntry.conformsToSchema(null, false, false, false,
- invalidReason))
- {
- throw new DirectoryException(DirectoryServer.getServerErrorResultCode(),
- ERR_MODIFYDN_ABORTED_BY_SUBORDINATE_SCHEMA_ERROR.get(oldDN, newDN, invalidReason));
- }
- }
- }
-
- dn2id.remove(txn, oldDN);
-
- // Remove old ID from id2entry and put the new entry (old entry with new DN) in id2entry.
- if (!newID.equals(oldID))
- {
- id2entry.remove(txn, oldID);
- }
-
- // Update any referral records.
- dn2uri.deleteEntry(txn, oldEntry);
-
- tail.next = new MovedEntry(newID, newEntry, !newID.equals(oldID));
-
- if (!newID.equals(oldID))
- {
- id2childrenCount.deleteCount(txn, oldID);
-
- // Reindex the entry with the new ID.
- removeEntryFromIndexes(buffer, oldEntry, oldID);
- }
- else if (!modifications.isEmpty())
- {
- // Update the indexes.
- indexModifications(buffer, oldEntry, newEntry, oldID, modifications);
- }
-
- // Remove the entry from the entry cache.
- EntryCache<?> entryCache = DirectoryServer.getEntryCache();
- if (entryCache != null)
- {
- entryCache.removeEntry(oldDN);
- }
- }
-
/**
* Make a new DN for a subordinate entry of a renamed or moved entry.
*
@@ -2507,8 +2316,7 @@
long getNumberOfEntriesInBaseDN0(ReadableTransaction txn)
{
- final int baseDnIfExists = dn2id.get(txn, baseDN) != null ? 1 : 0;
- return id2childrenCount.getTotalCount(txn) + baseDnIfExists;
+ return id2childrenCount.getTotalCount(txn);
}
/**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Count.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Count.java
index 68613d6..04164c0 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Count.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Count.java
@@ -79,16 +79,30 @@
}
/**
- * Add a value to the counter associated to the given key
- * @param txn Database transaction
- * @param entryID The entryID identifying to the counter
+ * Updates the counter associated with the given key, but does not update the total count.
+ * <p>
+ * Implementation note: this method accepts a {@code null} entryID in order to eliminate null checks in client code.
+ * In particular, client code has to deal with the special case where a target entry does not have a parent because
+ * the target entry is a base entry within the backend.
+ *
+ * @param txn storage transaction
+ * @param entryID The entryID identifying to the counter, which may be
+ * {@code null} in which case calling this method has no effect.
* @param delta The value to add. Can be negative to decrease counter value.
*/
- void addDelta(WriteableTransaction txn, EntryID entryID, final long delta)
- {
- Reject.ifTrue(entryID.longValue() >= TOTAL_COUNT_ENTRY_ID.longValue(), "EntryID overflow.");
+ void updateCount(final WriteableTransaction txn, final EntryID entryID, final long delta) {
+ if (entryID != null)
+ {
+ addToCounter(txn, entryID, delta);
+ }
+ }
- addToCounter(txn, entryID, delta);
+ /**
+ * Updates the total count which should be the sum of all counters.
+ * @param txn storage transaction
+ * @param delta The value to add. Can be negative to decrease counter value.
+ */
+ void updateTotalCount(final WriteableTransaction txn, final long delta) {
addToCounter(txn, TOTAL_COUNT_ENTRY_ID, delta);
}
@@ -209,27 +223,24 @@
}
/**
- * Delete the counter associated to the given key
- * @param txn The transaction
+ * Removes the counter associated to the given key, but does not update the total count.
+ * @param txn storage transaction
* @param entryID The entryID identifying the counter
* @return Value of the counter before it's deletion.
*/
- long deleteCount(WriteableTransaction txn, EntryID entryID)
- {
+ long removeCount(final WriteableTransaction txn, final EntryID entryID) {
long counterValue = 0;
- try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(getName())) {
+ try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(getName())) {
final ByteSequence encodedEntryID = getKeyFromEntryID(entryID);
if (cursor.positionToKeyOrNext(encodedEntryID)) {
- while (cursor.getKey().startsWith(encodedEntryID))
- {
+ // Iterate over and remove all the thread local shards
+ while (cursor.isDefined() && cursor.getKey().startsWith(encodedEntryID)) {
counterValue += cursor.getValue().asReader().getLong();
- txn.delete(getName(), cursor.getKey());
+ cursor.delete();
cursor.next();
}
}
}
- addToCounter(txn, TOTAL_COUNT_ENTRY_ID, -counterValue);
-
return counterValue;
}
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java
index 7b70fb1..281ca9e 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java
@@ -30,6 +30,7 @@
import static org.forgerock.util.Utils.*;
import static org.opends.messages.BackendMessages.*;
import static org.opends.server.core.DirectoryServer.*;
+import static org.opends.server.backends.pluggable.CursorTransformer.transformKeysAndValues;
import java.io.IOException;
import java.io.OutputStream;
@@ -45,6 +46,7 @@
import org.forgerock.opendj.ldap.ByteString;
import org.forgerock.opendj.ldap.ByteStringBuilder;
import org.forgerock.opendj.ldap.DecodeException;
+import org.forgerock.util.Function;
import org.forgerock.util.Reject;
import org.opends.server.api.CompressedSchema;
import org.opends.server.backends.pluggable.spi.Cursor;
@@ -65,6 +67,24 @@
{
private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+ /** Transforms cursor keys into EntryIDs. */
+ private static final Function<ByteString, EntryID, Exception> TO_ENTRY_ID =
+ new Function<ByteString, EntryID, Exception>() {
+ @Override
+ public EntryID apply(ByteString value) throws Exception {
+ return new EntryID(value);
+ }
+ };
+
+ /** Transforms cursor values into Entry objects. */
+ private final CursorTransformer.ValueTransformer<ByteString, ByteString, Entry, Exception> TO_ENTRY =
+ new CursorTransformer.ValueTransformer<ByteString, ByteString, Entry, Exception>() {
+ @Override
+ public Entry transform(ByteString key, ByteString value) throws Exception {
+ return get0(value);
+ }
+ };
+
/** Parameters for compression and encryption. */
private DataConfig dataConfig;
@@ -372,6 +392,10 @@
throw new DirectoryException(DirectoryServer.getServerErrorResultCode(), ERR_ENTRY_DATABASE_CORRUPT.get(entryID));
}
}
+
+ Cursor<EntryID, Entry> openCursor(ReadableTransaction txn)
+ {
+ return transformKeysAndValues(txn.openCursor(getName()), TO_ENTRY_ID, TO_ENTRY);
}
/**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java
index 8c491f8..875b0b1 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java
@@ -1704,6 +1704,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
key = value = null;
@@ -1903,6 +1909,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
key = value = null;
@@ -1994,6 +2006,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
key = null;
@@ -2116,6 +2134,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
closeSilently(orderedCursors);
@@ -2231,8 +2255,7 @@
totalNumberOfEntries++;
}
}
- // -1 because baseDN is not counted
- id2count.importPutTotalCount(asImporter(id2CountChunk), Math.max(0, totalNumberOfEntries - 1));
+ id2count.importPutTotalCount(asImporter(id2CountChunk), Math.max(0, totalNumberOfEntries));
new ChunkCopierTask(reporter, id2CountChunk, id2count.getName(), importer).call();
return null;
@@ -2508,6 +2531,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
// nothing to do
@@ -3282,6 +3311,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public long getNbBytesRead()
{
return bytesRead;
@@ -3336,6 +3371,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ delegate.delete();
+ }
+
+ @Override
public void close()
{
delegate.close();
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java
index f9f278e..02923cc 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java
@@ -127,6 +127,14 @@
}
@Override
+ public void delete()
+ {
+ traceEnter("delete");
+ cursor.delete();
+ traceLeave("delete");
+ }
+
+ @Override
public boolean isDefined()
{
traceEnter("isDefined");
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/SequentialCursor.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/SequentialCursor.java
index 6bc2276..f7a8f4f 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/SequentialCursor.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/SequentialCursor.java
@@ -68,7 +68,17 @@
*/
V getValue() throws NoSuchElementException;
+ /**
+ * Deletes the record on which this cursor is currently positioned. This method does not alter the position of the
+ * cursor. In particular, {@link #next()} must be called in order to point to the next record. The behavior of
+ * methods {@link #getKey()} and {@link #getValue()} after this method returns is undefined.
+ *
+ * @throws NoSuchElementException if the cursor is not defined.
+ * @throws UnsupportedOperationException if the cursor implementation does not support updates.
+ */
+ void delete() throws NoSuchElementException, UnsupportedOperationException;
+
/** {@inheritDoc} */
@Override
void close();
-}
\ No newline at end of file
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DefaultIndexTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DefaultIndexTest.java
index f3065ba..c246230 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DefaultIndexTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/DefaultIndexTest.java
@@ -170,6 +170,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ it.remove();
+ }
+
+ @Override
public boolean isDefined()
{
return current != null;
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2CountTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2CountTest.java
index b4fce3c..68419de 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2CountTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2CountTest.java
@@ -131,18 +131,18 @@
@Test
public void testDeleteCounterDecrementTotalCounter() throws Exception
{
- addDelta(id(0), 1024);
- addDelta(id(1), 1024);
- addDelta(id(2), 1024);
- addDelta(id(3), 1024);
+ updateCount(id(0), 1024);
+ updateCount(id(1), 1024);
+ updateCount(id(2), 1024);
+ updateCount(id(3), 1024);
assertThat(getTotalCounter()).isEqualTo(4096);
- assertThat(deleteCount(id(0))).isEqualTo(1024);
+ assertThat(removeCount(id(0))).isEqualTo(1024);
assertThat(getTotalCounter()).isEqualTo(3072);
- assertThat(deleteCount(id(1))).isEqualTo(1024);
- assertThat(deleteCount(id(2))).isEqualTo(1024);
- assertThat(deleteCount(id(3))).isEqualTo(1024);
+ assertThat(removeCount(id(1))).isEqualTo(1024);
+ assertThat(removeCount(id(2))).isEqualTo(1024);
+ assertThat(removeCount(id(3))).isEqualTo(1024);
assertThat(getTotalCounter()).isEqualTo(0);
}
@@ -171,7 +171,7 @@
@Override
public Void call() throws Exception
{
- addDelta(key, delta);
+ updateCount(key, delta);
return null;
}
});
@@ -179,26 +179,29 @@
return expected;
}
- private long deleteCount(final EntryID key) throws Exception {
+ private long removeCount(final EntryID key) throws Exception {
final PromiseImpl<Long, NeverThrowsException> l = PromiseImpl.create();
storage.write(new WriteOperation()
{
@Override
public void run(WriteableTransaction txn) throws Exception
{
- l.handleResult(id2Count.deleteCount(txn, key));
+ final long delta = id2Count.removeCount(txn, key);
+ id2Count.updateTotalCount(txn, -delta);
+ l.handleResult(delta);
}
});
return l.get();
}
- private void addDelta(final EntryID key, final long delta) throws Exception {
+ private void updateCount(final EntryID key, final long delta) throws Exception {
storage.write(new WriteOperation()
{
@Override
public void run(WriteableTransaction txn) throws Exception
{
- id2Count.addDelta(txn, key, delta);
+ id2Count.updateCount(txn, key, delta);
+ id2Count.updateTotalCount(txn, delta);
}
});
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/OnDiskMergeImporterTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/OnDiskMergeImporterTest.java
index ba482ee..f614a92 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/OnDiskMergeImporterTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/OnDiskMergeImporterTest.java
@@ -420,6 +420,12 @@
}
@Override
+ public void delete() throws NoSuchElementException, UnsupportedOperationException
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public void close()
{
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
index ae9517c..b3f18ee 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java
@@ -937,7 +937,8 @@
importConf.setThreadCount(0);
backend.importLDIF(importConf, DirectoryServer.getInstance().getServerContext());
}
- assertEquals(rejectedEntries.size(), 0, "No entries should be rejected");
+ assertEquals(rejectedEntries.size(), 0,
+ "No entries should be rejected. Content was:\n" + rejectedEntries.toString());
backend.openBackend();
assertEquals(backend.getEntryCount(), ldifNumberOfEntries, "Not enough entries in DIT.");
@@ -1111,8 +1112,10 @@
@Override
public void run(WriteableTransaction txn) throws Exception
{
- id2ChildrenCount.deleteCount(txn, peopleID);
- id2ChildrenCount.addDelta(txn, peopleID, 1);
+ final long delta = id2ChildrenCount.removeCount(txn, peopleID);
+ id2ChildrenCount.updateTotalCount(txn, -delta);
+ id2ChildrenCount.updateCount(txn, peopleID, 1);
+ id2ChildrenCount.updateTotalCount(txn, 1);
}
});
--
Gitblit v1.10.0