From 3259fdc435a6ed8687e9572f7f05285caa47da2a Mon Sep 17 00:00:00 2001
From: Yannick Lecaillez <yannick.lecaillez@forgerock.com>
Date: Mon, 14 Mar 2016 14:05:08 +0000
Subject: [PATCH] OPENDJ-2719: PDB entries cannot be larger than 4MB.
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pdb/PDBStorage.java | 56 +++++--------
opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java | 136 ++++++++++++++++++++++++++++++++++
2 files changed, 158 insertions(+), 34 deletions(-)
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 1096953..9dda8be 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
@@ -39,9 +39,7 @@
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
-import java.util.Queue;
import java.util.Set;
-import java.util.concurrent.ConcurrentLinkedDeque;
import org.forgerock.i18n.LocalizableMessage;
import org.forgerock.i18n.slf4j.LocalizedLogger;
@@ -106,7 +104,7 @@
private static final int BUFFER_SIZE = 16 * 1024;
/** PersistIt implementation of the {@link Cursor} interface. */
- private static final class CursorImpl implements Cursor<ByteString, ByteString>
+ private final class CursorImpl implements Cursor<ByteString, ByteString>
{
private ByteString currentKey;
private ByteString currentValue;
@@ -121,7 +119,7 @@
public void close()
{
// Release immediately because this exchange did not come from the txn cache
- exchange.getPersistitInstance().releaseExchange(exchange);
+ releaseExchange(exchange);
}
@Override
@@ -268,29 +266,18 @@
/** PersistIt implementation of the {@link Importer} interface. */
private final class ImporterImpl implements Importer
{
- private final Queue<Map<TreeName, Exchange>> allExchanges = new ConcurrentLinkedDeque<>();
private final ThreadLocal<Map<TreeName, Exchange>> exchanges = new ThreadLocal<Map<TreeName, Exchange>>()
{
@Override
protected Map<TreeName, Exchange> initialValue()
{
- final Map<TreeName, Exchange> value = new HashMap<>();
- allExchanges.add(value);
- return value;
+ return new HashMap<>();
}
};
@Override
public void close()
{
- for (Map<TreeName, Exchange> map : allExchanges)
- {
- for (Exchange exchange : map.values())
- {
- db.releaseExchange(exchange);
- }
- map.clear();
- }
PDBStorage.this.close();
}
@@ -304,11 +291,10 @@
private void createTree(final Transaction txn, final TreeName treeName)
{
- Exchange ex = null;
try
{
txn.begin();
- ex = getNewExchange(treeName, true);
+ getNewExchange(treeName, true);
txn.commit();
}
catch (PersistitException e)
@@ -318,7 +304,6 @@
finally
{
txn.end();
- releaseExchangeSilenty(ex);
}
}
@@ -339,15 +324,6 @@
finally
{
txn.end();
- releaseExchangeSilenty(ex);
- }
- }
-
- private void releaseExchangeSilenty(Exchange ex)
- {
- if ( ex != null)
- {
- db.releaseExchange(ex);
}
}
@@ -466,7 +442,7 @@
finally
{
exchanges.values().remove(ex);
- db.releaseExchange(ex);
+ releaseExchange(ex);
}
}
@@ -588,7 +564,7 @@
}
finally
{
- db.releaseExchange(ex);
+ releaseExchange(ex);
}
}
@@ -608,7 +584,7 @@
{
for (final Exchange ex : exchanges.values())
{
- db.releaseExchange(ex);
+ releaseExchange(ex);
}
exchanges.clear();
}
@@ -663,7 +639,7 @@
}
finally
{
- db.releaseExchange(ex);
+ releaseExchange(ex);
}
}
@@ -698,9 +674,21 @@
}
}
- private Exchange getNewExchange(final TreeName treeName, final boolean create) throws PersistitException
+ Exchange getNewExchange(final TreeName treeName, final boolean create) throws PersistitException
{
- return db.getExchange(volume, treeName.toString(), create);
+ final Exchange ex = db.getExchange(volume, treeName.toString(), create);
+ ex.setMaximumValueSize(Value.MAXIMUM_SIZE);
+ return ex;
+ }
+
+ void releaseExchange(Exchange ex)
+ {
+ // Don't keep exchanges with enlarged value - let them be GC'd.
+ // This is also done internally by Persistit in TransactionPlayer line 197.
+ if (ex.getValue().getEncodedBytes().length < Value.DEFAULT_MAXIMUM_SIZE)
+ {
+ db.releaseExchange(ex);
+ }
}
private StorageImpl newStorageImpl() {
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java
new file mode 100644
index 0000000..acb48d2
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pdb/PDBStorageTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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 2016 ForgeRock AS.
+ */
+package org.opends.server.backends.pdb;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+import static org.opends.server.ConfigurationMock.*;
+import static org.opends.server.util.StaticUtils.*;
+import static org.forgerock.opendj.ldap.ByteString.*;
+
+import org.forgerock.opendj.config.server.ConfigException;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.TestCaseUtils;
+import org.opends.server.admin.std.server.PDBBackendCfg;
+import org.opends.server.backends.pluggable.spi.AccessMode;
+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.MemoryQuota;
+import org.opends.server.core.ServerContext;
+import org.opends.server.extensions.DiskSpaceMonitor;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import com.persistit.Exchange;
+
+public class PDBStorageTest extends DirectoryServerTestCase
+{
+ private final TreeName treeName = new TreeName("dc=test", "test");
+ private PDBStorage storage;
+
+ @BeforeClass
+ public static void startServer() throws Exception
+ {
+ TestCaseUtils.startServer();
+ }
+
+ @BeforeMethod
+ public void setUp() throws ConfigException
+ {
+ ServerContext serverContext = mock(ServerContext.class);
+ when(serverContext.getMemoryQuota()).thenReturn(new MemoryQuota());
+ when(serverContext.getDiskSpaceMonitor()).thenReturn(mock(DiskSpaceMonitor.class));
+
+ storage = new PDBStorage(createBackendCfg(), serverContext);
+ storage.open(AccessMode.READ_WRITE);
+ }
+
+ @AfterMethod
+ public void tearDown()
+ {
+ storage.close();
+ }
+
+ @Test
+ public void testCanAddLargeValues() throws Exception
+ {
+ storage.write(new WriteOperation()
+ {
+ private final TreeName treeName = new TreeName("dc=test", "test");
+
+ @Override
+ public void run(WriteableTransaction txn) throws Exception
+ {
+ txn.openTree(treeName, true);
+ txn.put(treeName, valueOfUtf8("4mb"), valueOfBytes(new byte[4 * MB]));
+ txn.put(treeName, valueOfUtf8("32mb"), valueOfBytes(new byte[32 * MB]));
+ // 64Mb is the maximum allowed for value size. But Persistit has header reducing the payload.
+ txn.put(treeName, valueOfUtf8("64mb"), valueOfBytes(new byte[63 * MB]));
+ }
+ });
+ }
+
+ @Test
+ public void testExchangeWithSmallValuesAreReleasedToPool() throws Exception
+ {
+ final Exchange initial = storage.getNewExchange(treeName, true);
+ storage.releaseExchange(initial);
+
+ storage.write(new WriteOperation()
+ {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception
+ {
+ txn.put(treeName, valueOfUtf8("small"), valueOfBytes(new byte[512 * KB]));
+ }
+ });
+
+ assertThat(storage.getNewExchange(treeName, true)).isSameAs(initial);
+ }
+
+ @Test
+ public void testExchangeWithLargeValuesAreNotReleasedToPool() throws Exception
+ {
+ final Exchange initial = storage.getNewExchange(treeName, true);
+ storage.releaseExchange(initial);
+
+ storage.write(new WriteOperation()
+ {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception
+ {
+ txn.put(treeName, valueOfUtf8("small"), valueOfBytes(new byte[16 * MB]));
+ }
+ });
+
+ assertThat(storage.getNewExchange(treeName, true)).isNotSameAs(initial);
+ }
+
+ protected PDBBackendCfg createBackendCfg()
+ {
+ PDBBackendCfg backendCfg = legacyMockCfg(PDBBackendCfg.class);
+ when(backendCfg.getBackendId()).thenReturn("PDBStorageTest");
+ when(backendCfg.getDBDirectory()).thenReturn("PDBStorageTest");
+ when(backendCfg.getDBDirectoryPermissions()).thenReturn("755");
+ when(backendCfg.getDBCacheSize()).thenReturn(0L);
+ when(backendCfg.getDBCachePercent()).thenReturn(20);
+ return backendCfg;
+ }
+
+}
--
Gitblit v1.10.0