From 97dbf50dd56ee7640411ac61e6aa288f208a4375 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Tue, 01 Sep 2026 08:54:30 +0000
Subject: [PATCH] [#874] Grant the offline tools a read-only JDBC transaction instead of refusing it (#880)
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VLVIndex.java | 23 +++++--
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 81 +++++++++++++++++++++++++++
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PluggableBackendImplTestCase.java | 34 +++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 29 +++++++++
4 files changed, 157 insertions(+), 10 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
index d1d8055..c94088c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
@@ -1043,6 +1043,16 @@
}
}
}
+ /**
+ * A transaction able to write, unless the storage was opened read-only: then it may open an existing tree and
+ * read it, and every mutating operation throws {@link ReadOnlyStorageException} instead.
+ * <p>
+ * The mode is checked per operation rather than refused here, because {@code RootContainer.open(AccessMode)}
+ * asks for a write transaction even in read-only mode - that is where it opens the compressed schema and the
+ * entry containers - so refusing to hand one out failed the offline {@code export-ldif}, {@code verify-index}
+ * and {@code backendstat} before they read anything (#874). Both other storages of this server already have
+ * this shape: {@code PDBStorage.ReadOnlyStorageImpl} and {@code CASStorage.TransactionImpl.checkReadOnly()}.
+ */
private final class WriteableTransactionTransactionImpl extends ReadableTransactionImpl implements WriteableTransaction {
// Shared by every table this transaction stamps: opening a backend opens all its trees,
@@ -1052,10 +1062,17 @@
public WriteableTransactionTransactionImpl(Connection con) {
super(con);
- if (!accessMode.isWriteable()) {
+ //captured once rather than read per operation: the access mode of the storage is mutable state -
+ //ImporterImpl reopens the storage READ_WRITE under its caller - and a transaction has to keep the mode
+ //it was created with. It also drives isReadOnly, so that a cursor this transaction opens refuses
+ //delete() as well.
+ isReadOnly = !accessMode.isWriteable();
+ }
+
+ void checkReadOnly() {
+ if (isReadOnly) {
throw new ReadOnlyStorageException();
}
- isReadOnly = false;
}
boolean isExistsTable(TreeName treeName) {
@@ -1096,6 +1113,7 @@
@Override
public void openTree(TreeName treeName, boolean createOnDemand) {
if (createOnDemand) {
+ checkReadOnly();
if (!isExistsTable(treeName)) {
try (final PreparedStatement statement=con.prepareStatement("create table "+getTableName(treeName)+" ("+getTableDialect()+")")){
execute(statement);
@@ -1158,6 +1176,7 @@
}
public void clearTree(TreeName treeName) {
+ checkReadOnly();
try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName))){
execute(statement);
con.commit();
@@ -1168,6 +1187,7 @@
@Override
public void deleteTree(TreeName treeName) {
+ checkReadOnly();
if (isExistsTable(treeName)) {
try (final PreparedStatement statement = con.prepareStatement("drop table " + getTableName(treeName))) {
execute(statement);
@@ -1183,6 +1203,7 @@
@Override
public void put(TreeName treeName, ByteSequence key, ByteSequence value) {
+ checkReadOnly();
try {
upsert(treeName, key, value);
} catch (SQLException e) {
@@ -1250,6 +1271,9 @@
@Override
public boolean update(TreeName treeName, ByteSequence key, UpdateFunction f) {
+ //checked before the read, so that a read-only transaction reports the mode rather than the value it
+ //computed being equal to the stored one
+ checkReadOnly();
final ByteString oldValue=read(treeName,key);
final ByteSequence newValue=f.computeNewValue(oldValue);
if (Objects.equals(newValue, oldValue))
@@ -1266,6 +1290,7 @@
@Override
public boolean delete(TreeName treeName, ByteSequence key) {
+ checkReadOnly();
try (final PreparedStatement statement=con.prepareStatement("delete from "+getTableName(treeName)+" where h="+hashParam(con)+" and k=?")){
statement.setString(1,key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2,real2db(key.toByteArray()));
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 757aa99..ca84641 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
@@ -105,6 +105,7 @@
/** The storage associated with this index. */
private final Storage storage;
private final State state;
+ private final EntryContainer entryContainer;
/**
* A flag to indicate if this vlvIndex should be trusted to be consistent with the entries tree.
@@ -131,15 +132,8 @@
}
this.state = state;
+ this.entryContainer = entryContainer;
this.trusted = state.getIndexFlags(txn, getName()).contains(IndexFlag.TRUSTED);
- if (!trusted && entryContainer.getHighestEntryID(txn).longValue() == 0)
- {
- /*
- * If there are no entries in the entry container then there is no reason why this vlvIndex
- * can't be upgraded to trusted.
- */
- setTrusted(txn, true);
- }
this.config.addChangeListener(this);
}
@@ -163,6 +157,19 @@
void afterOpen(final WriteableTransaction txn, boolean createOnDemand) throws StorageRuntimeException
{
counter.open(txn, createOnDemand);
+ if (createOnDemand && !trusted && entryContainer.isEmpty(txn))
+ {
+ /*
+ * If there are no entries in the entry container then there is no reason why this vlvIndex
+ * can't be upgraded to trusted.
+ *
+ * Guarded by createOnDemand - which is accessMode.isWriteable() - and done here rather than in the
+ * constructor, as DefaultIndex.afterOpen() does: the transaction a read-only container opens is not
+ * allowed to write, so upgrading an untrusted index of an empty backend used to fail the offline tools
+ * on it instead of leaving the flag alone (#874).
+ */
+ setTrusted(txn, true);
+ }
}
@Override
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
index 9d7cdac..22e8ea1 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
@@ -22,6 +22,7 @@
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.ReadOnlyStorageException;
import org.opends.server.backends.pluggable.spi.ReadOperation;
import org.opends.server.backends.pluggable.spi.ReadableTransaction;
import org.opends.server.backends.pluggable.spi.TreeName;
@@ -320,6 +321,86 @@
}
}
+ /**
+ * A storage opened READ_ONLY must still hand out the write transaction {@code RootContainer.open()} asks for
+ * there - otherwise the offline export-ldif, verify-index and backendstat fail before reading anything - and
+ * that transaction must serve exactly what the open needs and nothing more: opening an existing tree, reads,
+ * cursors and record counts, while every mutation, including a delete through a cursor it opened, is
+ * refused (#874).
+ */
+ @Test
+ public void testReadOnlyTransactionReadsButRefusesWrites() throws Exception {
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null);
+ final TreeName tree = new TreeName("testReadOnlyTransaction", "tree");
+ final TreeName absent = new TreeName("testReadOnlyTransaction", "absent");
+ try {
+ storage.open(AccessMode.READ_WRITE);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.openTree(tree, true);
+ txn.put(tree, key(0), value(0));
+ txn.put(tree, key(1), value(1));
+ }
+ });
+ storage.close();
+
+ storage.open(AccessMode.READ_ONLY);
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ // what RootContainer.open() does through this transaction in read-only mode
+ txn.openTree(tree, false);
+ assertEquals(txn.read(tree, key(0)), value(0));
+ assertEquals(txn.getRecordCount(tree), 2);
+ try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(tree)) {
+ assertTrue(cursor.next());
+ assertEquals(cursor.getKey(), key(0));
+ try {
+ cursor.delete();
+ fail("delete() through a cursor of a read-only transaction must fail");
+ } catch (UnsupportedOperationException expected) {}
+ }
+
+ assertReadOnly("openTree(createOnDemand)", () -> txn.openTree(absent, true));
+ assertReadOnly("put", () -> txn.put(tree, key(2), value(2)));
+ assertReadOnly("update", () -> txn.update(tree, key(0), old -> value(3)));
+ assertReadOnly("delete", () -> txn.delete(tree, key(0)));
+ assertReadOnly("deleteTree", () -> txn.deleteTree(tree));
+ }
+ });
+
+ // nothing above reached the database
+ storage.close();
+ storage.open(AccessMode.READ_WRITE);
+ storage.read(new ReadOperation<Void>() {
+ @Override
+ public Void run(ReadableTransaction txn) throws Exception {
+ assertEquals(txn.getRecordCount(tree), 2);
+ assertEquals(txn.read(tree, key(0)), value(0));
+ return null;
+ }
+ });
+ } finally {
+ try {
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.deleteTree(tree);
+ }
+ });
+ } catch (Exception ignored) {}
+ storage.close();
+ }
+ }
+
+ private static void assertReadOnly(String operation, Runnable mutation) {
+ try {
+ mutation.run();
+ fail(operation + " must fail on a read-only storage");
+ } catch (ReadOnlyStorageException expected) {}
+ }
+
/** Buffer-served repositioning relies on the database collating keys in unsigned byte order. */
@Test
public void testCursorKeyOrderIsUnsigned() throws Exception {
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 eef3190..6706f7b 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
@@ -1251,6 +1251,40 @@
}
}
+ /**
+ * export-ldif, verify-index and backendstat open a root container of their own, in READ_ONLY mode, when the
+ * backend is not already open - a stopped server or a disabled backend. RootContainer.open() asks the storage
+ * for a write transaction even in that mode, since that is where it opens the compressed schema and the entry
+ * containers, so a storage refusing to hand one out fails the three tools before they read anything (#874).
+ * <p>
+ * testReadOnly() above does not cover this: it expects a ReadOnlyStorageException and a storage that fails the
+ * open throws one too, from RootContainer.open() rather than from the write it is meant to be checking.
+ */
+ @Test
+ public void testOfflineToolsOpenBackendReadOnly() throws Exception
+ {
+ // Put the backend offline, so that the tools open a read-only root container of their own
+ backend.finalizeBackend();
+ try
+ {
+ final ByteArrayOutputStream exported = new ByteArrayOutputStream();
+ try (final LDIFExportConfig exportConfig = new LDIFExportConfig(exported))
+ {
+ backend.exportLDIF(exportConfig);
+ }
+ assertThat(exported.toString(StandardCharsets.UTF_8.name())).contains(testBaseDN.toString());
+
+ final VerifyConfig verifyConfig = new VerifyConfig();
+ verifyConfig.setBaseDN(testBaseDN);
+ verifyConfig.addCompleteIndex("dn2id");
+ assertThat(backend.verifyBackend(verifyConfig)).isEqualTo(0);
+ }
+ finally
+ {
+ backend.openBackend();
+ }
+ }
+
@Test
public void test_issue_496() throws Exception {
int resultCode = TestCaseUtils.applyModifications(true,
--
Gitblit v1.10.0