diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java index e30c7481f8..920b866815 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/cassandra/CASStorage.java @@ -27,6 +27,7 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; +import java.util.regex.Pattern; import org.forgerock.i18n.LocalizableMessage; import org.forgerock.i18n.slf4j.LocalizedLogger; @@ -63,6 +64,7 @@ import com.datastax.oss.driver.api.core.cql.ResultSet; import com.datastax.oss.driver.api.core.cql.Row; import com.datastax.oss.driver.api.core.cql.Statement; +import com.datastax.oss.driver.api.core.servererrors.InvalidQueryException; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.LoadingCache; @@ -204,6 +206,13 @@ public void write(WriteOperation writeOperation) throws Exception { System.setProperty("datastax-java-driver.profiles."+profile+".basic.request.timeout", "30 seconds"); } } + // The wordings a server uses for a table or a keyspace that is not there. Matched rather than + // the exception type, which covers the whole INVALID protocol code: see namesAnAbsentTable(). + // "Undefined column name ..." deliberately does not match - that table exists. + static final Pattern ABSENT_TABLE=Pattern.compile( + "unconfigured (table|columnfamily)|(table|keyspace)[^,]* does not exist|undefined (table|keyspace)", + Pattern.CASE_INSENSITIVE); + private final class TransactionImpl implements ReadableTransaction,WriteableTransaction { final AccessMode accessMode; @@ -247,6 +256,57 @@ public long getRecordCount(TreeName treeName) { ).one().getLong(0); } + @Override + public boolean treeExists(TreeName treeName) { + // Every tree of this backend is a partition of the one table named after the backend id, + // so a tree has no existence apart from its rows: "exists" here means "holds at least one + // record". A LIMIT 1 lookup rather than getRecordCount() to avoid the full partition scan + // a count would run. + try { + return execute( + prepared.get("SELECT key FROM "+getTableName()+" WHERE baseDN=:baseDN and indexId=:indexId LIMIT 1").bind() + .setString("baseDN", treeName.getBaseDN()).setString("indexId", treeName.getIndexId()) + ).one()!=null; + }catch (InvalidQueryException e) { + // The backend's own table has not been created yet - a read-only open of a backend + // that was never written, where openTree() creates nothing - so none of its trees + // can exist either. The driver reports it from prepare() as much as from execute(). + // + // Narrowly, twice over, because calling a populated tree absent would have the + // compressed schema restart its token allocation from zero and overwrite the + // definitions the entries already written were encoded with (#873): + // + // - InvalidQueryException carries the whole INVALID protocol code, so the rejection + // has to name an absent table or keyspace. An undefined column, say, means the + // table is there with a shape this backend did not write, and answering "absent" + // for it would be that same corruption; + // - and only a read-only open may answer it at all. A writeable open has just run + // CREATE TABLE IF NOT EXISTS through openTree(), so a rejection there is a fault + // and must fail the open, as it did before this method existed and loadTrees() + // opened its cursor unconditionally. This is also where a table a coordinator has + // not caught up with lands - what a rolling upgrade produces, since schema + // agreement is never reached in a mixed-version cluster - and it fails loudly + // rather than being taken for a table that was never created. + if (accessMode.isWriteable() || !namesAnAbsentTable(e)) { + throw e; + } + return false; + } + } + + /** + * Whether a rejected query says that the table or the keyspace is not there, as opposed to + * anything else the INVALID protocol code covers. The driver offers nothing finer than the + * server's own message - the code is one value for the whole bucket - so the wordings of + * the server are matched, across the versions that changed them ("unconfigured + * columnfamily" became "unconfigured table", and a keyspace is reported as not existing). + * A wording that is not among them is not read as an absent table: it fails the open, which + * is the side to err on. + */ + private boolean namesAnAbsentTable(InvalidQueryException e) { + return e.getMessage()!=null && ABSENT_TABLE.matcher(e.getMessage()).find(); + } + @Override public void deleteTree(TreeName treeName) { checkReadOnly(); 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 0309d69f13..f1b9419148 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 @@ -205,27 +205,62 @@ public void close() { unstampableTrees.clear(); } + // The trees this storage has taken an interest in, and the tables they map to. listTrees() - + // and through it removeStorageFiles() - reads this, so a tree only belongs here once this + // backend uses it: see toTableName() below for the trees that are merely asked about. final LoadingCache tree2table = Caffeine.newBuilder() - .build(treeName -> { - try { - final MessageDigest md = MessageDigest.getInstance("SHA-224"); - final byte[] messageDigest = md.digest(treeName.toString().getBytes()); - final StringBuilder hashtext = new StringBuilder(56); - for (byte b : messageDigest) { - String hex = Integer.toHexString(0xff & b); - if (hex.length() == 1) hashtext.append('0'); - hashtext.append(hex); - } - return "opendj_" + hashtext; - } catch (NoSuchAlgorithmException e) { - throw new RuntimeException(e); - } - }); + .build(JDBCStorage::toTableName); + + /** + * The table a tree name maps to. A pure function of the name, so that a tree can be read + * without being entered into tree2table: the compressed schema reads the tree its definitions + * used to be shared under (#873), a tree this backend does not own, and removeStorageFiles() + * drops every table tree2table names. + *

+ * Which of the two a statement takes therefore says who owns the tree it names: a path that + * creates or writes one - openTree(), clearTree(), deleteTree(), put(), update(), delete() - + * takes the enrolling {@link #getTableName(TreeName)}, and a read-only path - read(), + * getRecordCount(), isExistsTable() and the cursor - takes {@link #readTableName(TreeName)}, + * which computes this only for a tree that is not enrolled already. Every tree this backend + * owns passes through openTree(name, true) as it is opened, so listTrees() still names the + * complete owned set. + */ + static String toTableName(TreeName treeName) { + try { + final MessageDigest md = MessageDigest.getInstance("SHA-224"); + final byte[] messageDigest = md.digest(treeName.toString().getBytes()); + final StringBuilder hashtext = new StringBuilder(56); + for (byte b : messageDigest) { + String hex = Integer.toHexString(0xff & b); + if (hex.length() == 1) hashtext.append('0'); + hashtext.append(hex); + } + return "opendj_" + hashtext; + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } + } String getTableName(TreeName treeName) { return tree2table.get(treeName); } + /** + * The table a tree name maps to, for a statement that only reads it. Answered from the memo of + * {@link #getTableName(TreeName)} where the tree is in it, and computed without being put there + * otherwise. + *

+ * Every tree this backend owns is enrolled as it is opened, so the per-entry read path stays a + * map lookup: {@link #toTableName(TreeName)} takes a JCA provider lookup and a digest per call, + * which read() would otherwise pay for every entry of every search. Only a tree this backend + * does not own - the shared compressed schema tree the migration of #873 reads - is computed, + * twice per open of the backend. + */ + String readTableName(TreeName treeName) { + final String enrolled=tree2table.getIfPresent(treeName); + return enrolled!=null ? enrolled : toTableName(treeName); + } + /** * The form a catalog pattern has to take to match an identifier this backend created unquoted. * An unquoted identifier is folded when it is stored - to upper case on oracle, to lower case @@ -1305,7 +1340,7 @@ public ReadableTransactionImpl(Connection con) { @Override public ByteString read(TreeName treeName, ByteSequence key) { - try (final PreparedStatement statement=con.prepareStatement("select v from "+getTableName(treeName)+" where h="+hashParam(con)+" and k=?")){ + try (final PreparedStatement statement=con.prepareStatement("select v from "+readTableName(treeName)+" where h="+hashParam(con)+" and k=?")){ statement.setString(1,key2hash.get(ByteBuffer.wrap(key.toByteArray()))); statement.setBytes(2,real2db(key.toByteArray())); try(ResultSet rc=executeResultSet(statement)) { @@ -1323,13 +1358,47 @@ public Cursor openCursor(TreeName treeName) { @Override public long getRecordCount(TreeName treeName) { - try (final PreparedStatement statement=con.prepareStatement("select count(*) from "+getTableName(treeName)); + try (final PreparedStatement statement=con.prepareStatement("select count(*) from "+readTableName(treeName)); final ResultSet rc=executeResultSet(statement)){ return rc.next() ? rc.getLong(1) : 0; }catch (SQLException e) { throw new StorageRuntimeException(e); } } + + @Override + public boolean treeExists(TreeName treeName) { + return isExistsTable(treeName); + } + + // Readable, not writeable: the caller that asks about a tree this backend does not own is + // the compressed schema migration (#873), which probes the shared tree from the writeable + // transaction of RootContainer.open() but must not create or enrol it. Answering that from + // the readable transaction keeps the probe available to every reader, and costs nothing: + // the writeable one inherits it. + boolean isExistsTable(TreeName treeName) { + final String tableName = readTableName(treeName); + try { + final DatabaseMetaData metaData = con.getMetaData(); + // asked of the catalog by name: openTree(createOnDemand) calls this for every tree + // of the backend - about 25 of them for a stock suffix, on every open - and listing + // every table of the database each time costs the whole catalog once per tree, on a + // database this backend may well be sharing with something else + try (final ResultSet rs = metaData.getTables(null, null, + storedIdentifier(metaData, tableName), new String[]{"TABLE"})) { + while (rs.next()) { + // the name still has to be compared: "_" is a single-character wildcard in a + // metadata pattern, so "opendj_" also matches a table named "opendjX" + if (tableName.equalsIgnoreCase(rs.getString("TABLE_NAME"))) { + return true; + } + } + } + } catch (Exception e) { + throw new StorageRuntimeException(e); + } + return false; + } } /** * A transaction able to write, unless the storage was opened read-only: then it may open an existing tree and @@ -1405,30 +1474,6 @@ private boolean commitsBeforeDdl() { return driverName.contains("mysql") || driverName.contains("oracle"); } - boolean isExistsTable(TreeName treeName) { - final String tableName = getTableName(treeName); - try { - final DatabaseMetaData metaData = con.getMetaData(); - // asked of the catalog by name: openTree(createOnDemand) calls this for every tree - // of the backend - about 25 of them for a stock suffix, on every open - and listing - // every table of the database each time costs the whole catalog once per tree, on a - // database this backend may well be sharing with something else - try (final ResultSet rs = metaData.getTables(null, null, - storedIdentifier(metaData, tableName), new String[]{"TABLE"})) { - while (rs.next()) { - // the name still has to be compared: "_" is a single-character wildcard in a - // metadata pattern, so "opendj_" also matches a table named "opendjX" - if (tableName.equalsIgnoreCase(rs.getString("TABLE_NAME"))) { - return true; - } - } - } - } catch (Exception e) { - throw new StorageRuntimeException(e); - } - return false; - } - String getTableDialect() { if (driverNameOf(con).contains("oracle")) { return "h char(128),k raw(2000),v blob,primary key(h,k)"; @@ -1645,7 +1690,10 @@ static int compareKeys(byte[] key1, byte[] key2) { // repositioning transfer "fetchsize" rows over the network (#860). final class CursorImpl implements Cursor { final Connection con; + final TreeName treeName; final String tableName; + // the enrolling name, resolved once and only if this cursor ever deletes + String writeTableName; final boolean isReadOnly; final int batchSize=Math.max(1,Integer.getInteger("org.openidentityplatform.opendj.jdbc.fetchsize",1000)); final int initialBatchSize=Math.min(batchSize,Math.max(1,Integer.getInteger("org.openidentityplatform.opendj.jdbc.fetchsize.initial",32))); @@ -1662,7 +1710,10 @@ final class CursorImpl implements Cursor { public CursorImpl(boolean isReadOnly, Connection con, TreeName treeName) { this.isReadOnly=isReadOnly; this.con=con; - this.tableName=getTableName(treeName); + this.treeName=treeName; + // the read statements below take the non-enrolling name: a cursor is how the migration + // of #873 reads the shared tree, and reading a tree must not put it up for removal + this.tableName=readTableName(treeName); this.limitClause=((CachedConnection)con).parent.getClass().getName().contains("mysql") ? " limit ?,?" : " offset ? rows fetch next ? rows only"; } @@ -1743,7 +1794,12 @@ public void delete() throws NoSuchElementException, UnsupportedOperationExceptio if (isReadOnly) { throw new UnsupportedOperationException(); } - try (final PreparedStatement statement=con.prepareStatement("delete from "+tableName+" where h="+hashParam(con)+" and k=?")){ + if (writeTableName==null) { + // the enrolling name, unlike the read statements above: this writes to the tree, so + // it is one this backend owns, and removeStorageFiles() has to know about it + writeTableName=getTableName(treeName); + } + try (final PreparedStatement statement=con.prepareStatement("delete from "+writeTableName+" where h="+hashParam(con)+" and k=?")){ statement.setString(1,key2hash.get(ByteBuffer.wrap(db2real(currentKeyDb)))); statement.setBytes(2,currentKeyDb); execute(statement); 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 cc9b8700d3..4db4d9fcaf 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 @@ -427,6 +427,21 @@ public long getRecordCount(TreeName treeName) } } + @Override + public boolean treeExists(TreeName treeName) + { + // Deliberately not getOrOpenTree(): dbConfig() sets allowCreate, so asking the tree itself + // would create the very database whose absence is being tested. + try + { + return env.getDatabaseNames().contains(toDatabaseName(treeName)); + } + catch (DatabaseException e) + { + throw new StorageRuntimeException(e); + } + } + @Override public Cursor openCursor(final TreeName treeName) { @@ -550,6 +565,12 @@ public long getRecordCount(TreeName treeName) return delegate.getRecordCount(treeName); } + @Override + public boolean treeExists(TreeName treeName) + { + return delegate.treeExists(treeName); + } + @Override public void openTree(TreeName treeName, boolean createOnDemand) { @@ -638,6 +659,13 @@ public long getRecordCount(TreeName treeName) { return 0; } + + @Override + public boolean treeExists(TreeName treeName) + { + // No environment was ever opened, so nothing is stored. + return false; + } } private WriteableTransaction newWriteableTransaction(Transaction 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 ea233952de..e99db989b4 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 @@ -467,6 +467,12 @@ public long getRecordCount(TreeName treeName) } } + @Override + public boolean treeExists(TreeName treeName) + { + return PDBStorage.this.treeExists(treeName); + } + @Override public Cursor openCursor(final TreeName treeName) { @@ -690,6 +696,12 @@ public long getRecordCount(TreeName treeName) return delegate.getRecordCount(treeName); } + @Override + public boolean treeExists(TreeName treeName) + { + return delegate.treeExists(treeName); + } + @Override public void openTree(TreeName treeName, boolean createOnDemand) { @@ -819,6 +831,13 @@ public long getRecordCount(TreeName treeName) return 0; } + @Override + public boolean treeExists(TreeName treeName) + { + // No volume was ever opened, so nothing is stored. + return false; + } + @Override public T read(ReadOperation operation) throws Exception { @@ -832,6 +851,27 @@ public void write(WriteOperation operation) throws Exception } } + /** + * Tells whether the volume holds a tree of that name. Deliberately not implemented by asking for + * an {@link Exchange}: {@code getExchange(volume, name, true)} would create the tree being tested, + * and with {@code false} Persistit reports its absence by throwing. + */ + boolean treeExists(final TreeName treeName) + { + if (volume == null) + { + return false; + } + try + { + return volume.getTree(treeName.toString(), false) != null; + } + catch (PersistitException e) + { + throw new StorageRuntimeException(e); + } + } + Exchange getNewExchange(final TreeName treeName, final boolean create) throws PersistitException { final Exchange ex = db.getExchange(volume, treeName.toString(), create); 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 5b537910e8..efb49c5297 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 @@ -1628,6 +1628,12 @@ public Cursor openCursor(TreeName treeName) throw new UnsupportedOperationException(); } + @Override + public boolean treeExists(TreeName treeName) + { + throw new UnsupportedOperationException(); + } + @Override public long getRecordCount(TreeName treeName) { @@ -4207,6 +4213,12 @@ public long getRecordCount(TreeName treeName) return counter; } + @Override + public boolean treeExists(TreeName treeName) + { + throw new UnsupportedOperationException(); + } + @Override public void openTree(TreeName name, boolean createOnDemand) { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/PersistentCompressedSchema.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/PersistentCompressedSchema.java index af484abb73..c3a9f42879 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/PersistentCompressedSchema.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/PersistentCompressedSchema.java @@ -13,10 +13,12 @@ * * Copyright 2008-2009 Sun Microsystems, Inc. * Portions Copyright 2013-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.pluggable; import static org.opends.messages.BackendMessages.*; +import static org.opends.server.util.StaticUtils.stackTraceToSingleLineString; import java.io.IOException; import java.util.Collection; @@ -32,6 +34,7 @@ import org.opends.server.api.CompressedSchema; import org.opends.server.backends.pluggable.spi.AccessMode; import org.opends.server.backends.pluggable.spi.Cursor; +import org.opends.server.backends.pluggable.spi.ReadableTransaction; import org.opends.server.backends.pluggable.spi.Storage; import org.opends.server.backends.pluggable.spi.StorageRuntimeException; import org.opends.server.backends.pluggable.spi.TreeName; @@ -55,14 +58,40 @@ final class PersistentCompressedSchema extends CompressedSchema /** The name of the tree used to store compressed object class set definitions. */ private static final String DB_NAME_OC = "compressed_object_classes"; - /** The compressed attribute description schema tree. */ - private static final TreeName adTreeName = new TreeName("compressed_schema", DB_NAME_AD); - /** The compressed object class set schema tree. */ - private static final TreeName ocTreeName = new TreeName("compressed_schema", DB_NAME_OC); + /** + * The tree prefix every backend shared before the definitions were separated per backend. + *

+ * Every other tree of a backend is named from {@link EntryContainer#getTreePrefix()} and so + * carries its base DN, but these two belong to the backend rather than to any one of its base + * DNs and were named from a literal. That is harmless where a storage holds a single backend - + * JE and PDB give each its own directory, and the Cassandra backend names its table after the + * backend id - but the JDBC backend derives its table name from the tree name alone, so two + * JDBC backends addressing one database met in this pair of tables. Each allocated tokens from + * the size of its own in-memory map under its own lock, so both handed out the same token for + * different attribute descriptions and overwrote each other's definitions; after a restart the + * entries of the losing backend decoded as the wrong attributes, silently (issue #873). + *

+ * Kept only to be read: {@link #load} migrates what it finds here into the backend's own trees + * and never writes to it again. + */ + private static final String LEGACY_TREE_PREFIX = "compressed_schema"; + private static final TreeName LEGACY_AD_TREE_NAME = new TreeName(LEGACY_TREE_PREFIX, DB_NAME_AD); + private static final TreeName LEGACY_OC_TREE_NAME = new TreeName(LEGACY_TREE_PREFIX, DB_NAME_OC); + + /** The compressed attribute description schema tree of this backend. */ + private final TreeName adTreeName; + /** The compressed object class set schema tree of this backend. */ + private final TreeName ocTreeName; + + /** The id of the backend these definitions belong to. */ + private final String backendId; /** The storage in which the trees are held. */ private final Storage storage; + /** How many definitions the migration copied, reported by {@link #reportMigration()} once committed. */ + private long migratedDefinitions; + private final ByteStringBuilder storeAttributeWriterBuffer = new ByteStringBuilder(); private final ASN1Writer storeAttributeWriter = ASN1.getWriter(storeAttributeWriterBuffer); private final ByteStringBuilder storeObjectClassesWriterBuffer = new ByteStringBuilder(); @@ -73,6 +102,9 @@ final class PersistentCompressedSchema extends CompressedSchema * * @param serverContext * The server context. + * @param backendId + * The id of the backend whose definitions are held, which qualifies the trees so that + * two backends sharing one database do not share one token space. * @param storage * A reference to the storage in which the trees will be held. * @param txn a non null transaction @@ -85,14 +117,41 @@ final class PersistentCompressedSchema extends CompressedSchema * If an error occurs while loading and processing the compressed * schema definitions. */ - PersistentCompressedSchema(ServerContext serverContext, final Storage storage, WriteableTransaction txn, - AccessMode accessMode) throws StorageRuntimeException, InitializationException + PersistentCompressedSchema(ServerContext serverContext, String backendId, final Storage storage, + WriteableTransaction txn, AccessMode accessMode) throws StorageRuntimeException, InitializationException { super(serverContext); this.storage = storage; + this.backendId = backendId; + final String treePrefix = treePrefix(backendId); + this.adTreeName = new TreeName(treePrefix, DB_NAME_AD); + this.ocTreeName = new TreeName(treePrefix, DB_NAME_OC); load(txn, accessMode.isWriteable()); } + /** + * Qualifies the legacy prefix with the backend id. {@link TreeName} documents that it assumes no + * name component contains a '/', and {@code TreeName.valueOf} splits the string form at the last + * one, so a prefix carrying a '/' would come back as a different name than it went in as. + * Nothing rules a '/' out of a backend id - ds-cfg-backend-id is a plain string - so it is + * escaped rather than passed through. The escape is reversible, so two distinct backend ids can + * never produce one prefix. + *

+ * The qualifier is the backend id rather than a base DN because this object is built once per + * {@link RootContainer}, before any entry container exists, and a backend holding two base DNs + * has no single prefix to borrow. The price is that these two trees, alone among the trees of a + * backend, do not follow the entries when a JDBC backend is deleted and re-created under another + * id over the same database: every other tree is named from its base DN and is found again, + * while these are not, leaving the token allocation to restart at zero over entries encoded with + * the definitions of the old id. Changing a backend id over populated storage needs an export + * and a re-import - as it always has on Cassandra, where the one table of a backend is named + * after the id and the entries do not survive the rename either. + */ + private static String treePrefix(String backendId) + { + return LEGACY_TREE_PREFIX + "_" + backendId.replace("%", "%25").replace("/", "%2F"); + } + @Override protected void storeAttribute(final byte[] encodedAttribute, final String attributeName, final Iterable attributeOptions) @@ -140,56 +199,219 @@ protected void storeObjectClasses(final byte[] encodedObjectClasses, private void load(WriteableTransaction txn, boolean shouldCreate) throws StorageRuntimeException, InitializationException { - txn.openTree(adTreeName, shouldCreate); - txn.openTree(ocTreeName, shouldCreate); + if (shouldCreate) + { + // Asked for only where the trees may be created. A read-only open must leave the storage as + // it found it, and passing the flag on would not: JEStorage.openTree ignores createOnDemand + // and reaches env.openDatabase() with setAllowCreate(true), so an offline export-ldif or + // verify-index of a backend that has not migrated yet would leave two empty databases behind. + // Nothing below needs the trees open - every read is guarded by treeExists(). + txn.openTree(adTreeName, true); + txn.openTree(ocTreeName, true); + } + + if (needsLegacyDefinitions(txn)) + { + if (shouldCreate) + { + migrateLegacyDefinitions(txn); + } + else + { + // Read-only: nothing may be written, so the legacy definitions are read where they lie. + // Loaded first, so that a token this backend has already migrated, and since re-used under + // its own prefix, decodes to its own definition. That settles the decode map only: + // CompressedSchema.loadAttributeToMaps keys the encode map by attribute description, so a + // legacy description displaced from a token stays in it and would encode to a token that + // now decodes to another attribute. Harmless only because nothing encodes during a + // read-only open - export-ldif and verify-index decode. + loadTrees(txn, LEGACY_OC_TREE_NAME, LEGACY_AD_TREE_NAME); + } + } + loadTrees(txn, ocTreeName, adTreeName); + } + + /** + * Tells whether the definitions under {@link #LEGACY_TREE_PREFIX} are still needed, either + * because this backend has not been opened since the upgrade that separated them, or because a + * previous migration did not run to completion - on a storage without transactions the copy can + * stop halfway. Once migrated, this backend's trees only ever grow, so they can no longer hold + * fewer records than the legacy ones and the question is settled without reading either tree. + *

+ * That invariant holds in one direction only. A version from before the separation resumes + * writing to the legacy pair, so after a downgrade and a second upgrade the counts can agree + * while the definitions behind them have diverged, and nothing is migrated. Downgrading across + * the separation is not a supported path - the legacy trees are left in place so that a backend + * still running an earlier version can go on reading them, not so that this one can go back. + *

+ * The question is settled again on every open, nothing recording that it has been answered + * before: two existence probes, and the record counts only where the legacy trees are still + * there. That is what not writing a marker of this backend's own into a database it may be + * sharing with another one costs. + *

+ * A backend created after the upgrade on a database that already holds legacy definitions copies + * them although it has no entries of its own. Deliberate: what it inherits is a consistent + * token-to-definition mapping and costs one copy, whereas asking instead whether its own trees + * are absent would answer "nothing to migrate" for the half-copied trees an interrupted + * migration leaves behind - the one case this method exists to catch. + */ + private boolean needsLegacyDefinitions(ReadableTransaction txn) + { + final long legacyAdCount = recordCount(txn, LEGACY_AD_TREE_NAME); + final long legacyOcCount = recordCount(txn, LEGACY_OC_TREE_NAME); + return (legacyAdCount > 0 || legacyOcCount > 0) + && (recordCount(txn, adTreeName) < legacyAdCount || recordCount(txn, ocTreeName) < legacyOcCount); + } + + /** The number of records of a tree, without asking a storage to materialize one that is absent. */ + private long recordCount(ReadableTransaction txn, TreeName treeName) + { + return txn.treeExists(treeName) ? txn.getRecordCount(treeName) : 0; + } + + private void migrateLegacyDefinitions(WriteableTransaction txn) throws InitializationException + { + migratedDefinitions = migrateTree(txn, LEGACY_AD_TREE_NAME, adTreeName) + + migrateTree(txn, LEGACY_OC_TREE_NAME, ocTreeName); + } + + private long migrateTree(WriteableTransaction txn, TreeName from, TreeName to) throws InitializationException + { + try + { + return copyMissingRecords(txn, from, to); + } + catch (final StorageRuntimeException e) + { + // Left with the type the storage gave it. The migration runs inside the WriteOperation of + // RootContainer.open(), and PDBStorage.write() decides by type what to do with what leaves + // it: a transaction conflict reaches its retry as a RollbackException only, so wrapping it + // here would turn a conflict that used to be replayed into a permanent failure of the open. + // Nothing is lost - a storage failure that is not a conflict still fails the open, through + // ERR_OPEN_ENV_FAIL, and leaves the trees as they were, since the transaction rolls back. + throw e; + } + catch (final Exception e) + { + logger.traceException(e); + // Deliberately fatal to the open. Loading no definitions at all would restart token + // allocation from zero and decode every entry written so far as the wrong attributes, + // without reporting anything. + throw new InitializationException( + ERR_COMPSCHEMA_CANNOT_MIGRATE.get(backendId, from, to, stackTraceToSingleLineString(e)), e); + } + } + + /** + * Reports a migration that has been committed, and reports nothing where none was needed. + *

+ * Called by {@link RootContainer#open} once the transaction the migration ran in has committed, + * and only then: a copy is undone by a rollback - a later failure of the same transaction, or a + * conflict PersistIt replays - and this line is the only evidence the migration path emits, so + * one standing for a copy that was rolled back, or repeated once per replay, would be worse than + * none at all. + */ + void reportMigration() + { + if (migratedDefinitions > 0) + { + logger.info(NOTE_COMPSCHEMA_MIGRATED, migratedDefinitions, backendId, + LEGACY_AD_TREE_NAME, LEGACY_OC_TREE_NAME, adTreeName, ocTreeName); + } + } + /** + * Copies the records of {@code from} that {@code to} does not already hold, and returns how many + * were copied. The legacy tree is left in place: on a shared database it may still be the only + * copy a backend that has not been upgraded yet has. + *

+ * A key already present in {@code to} is never overwritten, which is what makes this safe to + * re-run after an interrupted migration and safe against a legacy tree that a backend of an + * earlier version is still writing to. + */ + private long copyMissingRecords(WriteableTransaction txn, TreeName from, TreeName to) + { + if (!txn.treeExists(from)) + { + return 0; + } + long copied = 0; + try (Cursor cursor = txn.openCursor(from)) + { + while (cursor.next()) + { + final ByteString key = cursor.getKey(); + if (txn.read(to, key) == null) + { + txn.put(to, key, cursor.getValue()); + copied++; + } + } + } + return copied; + } + + /** + * Loads the object class set definitions and then the attribute description definitions of the + * provided pair of trees into the maps. A tree that does not exist contributes nothing: a + * read-only open creates none, and a storage that keeps one object per tree fails outright when + * asked to read one that was never written. + */ + private void loadTrees(ReadableTransaction txn, TreeName ocTree, TreeName adTree) throws InitializationException + { // Cursor through the object class database and load the object class set // definitions. At the same time, figure out the highest token value and // initialize the object class counter to one greater than that. - try (Cursor ocCursor = txn.openCursor(ocTreeName)) + if (txn.treeExists(ocTree)) { - while (ocCursor.next()) + try (Cursor ocCursor = txn.openCursor(ocTree)) { - final byte[] encodedObjectClasses = ocCursor.getKey().toByteArray(); - final ASN1Reader reader = ASN1.getReader(ocCursor.getValue()); - reader.readStartSequence(); - final List objectClassNames = new LinkedList<>(); - while (reader.hasNextElement()) + while (ocCursor.next()) { - objectClassNames.add(reader.readOctetStringAsString()); + final byte[] encodedObjectClasses = ocCursor.getKey().toByteArray(); + final ASN1Reader reader = ASN1.getReader(ocCursor.getValue()); + reader.readStartSequence(); + final List objectClassNames = new LinkedList<>(); + while (reader.hasNextElement()) + { + objectClassNames.add(reader.readOctetStringAsString()); + } + reader.readEndSequence(); + loadObjectClasses(encodedObjectClasses, objectClassNames); } - reader.readEndSequence(); - loadObjectClasses(encodedObjectClasses, objectClassNames); } - } - catch (final IOException e) - { - logger.traceException(e); - throw new InitializationException(ERR_COMPSCHEMA_CANNOT_DECODE_OC_TOKEN.get(e.getMessage()), e); + catch (final IOException e) + { + logger.traceException(e); + throw new InitializationException(ERR_COMPSCHEMA_CANNOT_DECODE_OC_TOKEN.get(e.getMessage()), e); + } } // Cursor through the attribute description database and load the attribute set definitions. - try (Cursor adCursor = txn.openCursor(adTreeName)) + if (txn.treeExists(adTree)) { - while (adCursor.next()) + try (Cursor adCursor = txn.openCursor(adTree)) { - final byte[] encodedAttribute = adCursor.getKey().toByteArray(); - final ASN1Reader reader = ASN1.getReader(adCursor.getValue()); - reader.readStartSequence(); - final String attributeName = reader.readOctetStringAsString(); - final List attributeOptions = new LinkedList<>(); - while (reader.hasNextElement()) + while (adCursor.next()) { - attributeOptions.add(reader.readOctetStringAsString()); + final byte[] encodedAttribute = adCursor.getKey().toByteArray(); + final ASN1Reader reader = ASN1.getReader(adCursor.getValue()); + reader.readStartSequence(); + final String attributeName = reader.readOctetStringAsString(); + final List attributeOptions = new LinkedList<>(); + while (reader.hasNextElement()) + { + attributeOptions.add(reader.readOctetStringAsString()); + } + reader.readEndSequence(); + loadAttribute(encodedAttribute, attributeName, attributeOptions); } - reader.readEndSequence(); - loadAttribute(encodedAttribute, attributeName, attributeOptions); } - } - catch (final IOException e) - { - logger.traceException(e); - throw new InitializationException(ERR_COMPSCHEMA_CANNOT_DECODE_AD_TOKEN.get(e.getMessage()), e); + catch (final IOException e) + { + logger.traceException(e); + throw new InitializationException(ERR_COMPSCHEMA_CANNOT_DECODE_AD_TOKEN.get(e.getMessage()), e); + } } } diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java index 94cb42d144..ad1dcd6380 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/RootContainer.java @@ -137,10 +137,13 @@ void open(final AccessMode accessMode) throws StorageRuntimeException, ConfigExc @Override public void run(WriteableTransaction txn) throws Exception { - compressedSchema = new PersistentCompressedSchema(serverContext, storage, txn, accessMode); + compressedSchema = new PersistentCompressedSchema(serverContext, backendId, storage, txn, accessMode); openAndRegisterEntryContainers(txn, config.getBaseDN(), accessMode); } }); + // after the write, never inside it: a compressed schema migration is only worth reporting + // once the transaction that copied it has committed, and a replayed operation runs twice + compressedSchema.reportMigration(); } catch(StorageRuntimeException e) { 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 172388ef62..42ff1dae71 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 @@ -287,6 +287,15 @@ public long getRecordCount(TreeName name) return count; } + @Override + public boolean treeExists(TreeName name) + { + traceEnter("treeExists", "name", name); + final boolean exists = txn.treeExists(name); + traceLeave("treeExists", "name", name, "exists", exists); + return exists; + } + @Override public Cursor openCursor(final TreeName name) { @@ -365,6 +374,15 @@ public long getRecordCount(TreeName name) return count; } + @Override + public boolean treeExists(TreeName name) + { + traceEnter("treeExists", "name", name); + final boolean exists = txn.treeExists(name); + traceLeave("treeExists", "name", name, "exists", exists); + return exists; + } + @Override public Cursor openCursor(final TreeName name) { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/ReadableTransaction.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/ReadableTransaction.java index 1f9ab9e02d..c84668754a 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/ReadableTransaction.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/ReadableTransaction.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2015 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.pluggable.spi; @@ -51,4 +52,26 @@ public interface ReadableTransaction * @return the number of key/value pairs in the provided tree. */ long getRecordCount(TreeName treeName); + + /** + * Returns whether the tree whose name is provided is present in the storage. + *

+ * This is not the same question as whether the tree is empty, and it cannot be answered by + * reading from the tree: a storage is free to materialize a tree on first access - the JE backend + * opens its databases with {@code setAllowCreate(true)} - or to reject the access outright, as the + * JDBC backend does when no table of that name exists. Callers that must distinguish "never + * written" from "written and since emptied", such as the compressed schema deciding whether it + * has anything to migrate, need this instead. + *

+ * A storage whose trees have no existence of their own cannot keep that distinction: the + * Cassandra backend holds every tree of a backend as a partition of the one table named after + * the backend id, so it answers whether the partition holds a record and a tree that was emptied + * reports itself absent. Nothing may be inferred from a {@code false} beyond "there is nothing + * to read here". + * + * @param treeName + * the tree name + * @return {@code true} if the tree exists, {@code false} otherwise + */ + boolean treeExists(TreeName treeName); } 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 a8d16b8795..eebfe842ef 100644 --- a/opendj-server-legacy/src/messages/org/opends/messages/backend.properties +++ b/opendj-server-legacy/src/messages/org/opends/messages/backend.properties @@ -1108,3 +1108,9 @@ ERR_SERVICE_DISCOVERY_CONFIG_MANAGER_LISTENER_615=Registering Service Discovery NOTE_IMPORT_MIGRATION_START_616=Migrating %s entries for base DN %s so that they are preserved by the partial import WARN_PSEARCH_BACKEND_UNAVAILABLE_617=The persistent search is being terminated because backend %s is \ no longer available +NOTE_COMPSCHEMA_MIGRATED_618=Migrated %d compressed schema definitions of backend '%s' from the shared trees \ + '%s' and '%s' to '%s' and '%s'. The shared trees are left untouched: on a storage where several backends address \ + one database, such as a JDBC backend, they may still hold the definitions of another backend +ERR_COMPSCHEMA_CANNOT_MIGRATE_619=The compressed schema definitions of backend '%s' could not be migrated from \ + the shared tree '%s' to '%s': %s. The backend cannot be opened, because its entries were encoded against the \ + definitions that were not migrated and would decode as the wrong attributes diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java index 9e784d3f0a..d73da222ae 100644 --- a/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/cassandra/TestCase.java @@ -42,6 +42,7 @@ import com.datastax.oss.driver.api.core.AllNodesFailedException; import com.datastax.oss.driver.api.core.CqlSession; import com.datastax.oss.driver.api.core.config.DriverConfigLoader; +import com.datastax.oss.driver.api.core.servererrors.InvalidQueryException; import java.net.InetSocketAddress; import java.util.NoSuchElementException; @@ -134,6 +135,68 @@ public void run(WriteableTransaction txn) throws Exception { }); } + /** A storage under a backend id of its own, so that its table is one no other test has created. */ + private CASStorage openStorage(String backendId, AccessMode accessMode) throws Exception { + final CASBackendCfg backendCfg = mockCfg(CASBackendCfg.class); + when(backendCfg.getBackendId()).thenReturn(backendId); + when(backendCfg.getDBDirectory()).thenReturn("CASTestCase"); + final CASStorage storage = new CASStorage(backendCfg, null); + storage.open(accessMode); + return storage; + } + + /** + * The compressed schema asks whether a tree is there before anything has been written (#873), so + * treeExists() has to answer for a table that openTree() has not created yet. It may only answer + * where nothing can follow from the answer: a read has nothing to lose, but a writeable + * transaction has just created the table through openTree(), so a query rejected there is a + * fault. Reporting a populated tree absent would have the compressed schema allocate its tokens + * from zero again and overwrite the definitions its entries were encoded with. + */ + @Test + public void testTreeExistsAnswersForAMissingTable() throws Exception { + final CASStorage storage = openStorage("CASTreeExists", AccessMode.READ_WRITE); + final TreeName tree = new TreeName("testTreeExists", "tree"); + try { + // the keyspace is there - open(READ_WRITE) creates it - but the table is not + assertFalse(storage.read(new ReadOperation() { + @Override + public Boolean run(ReadableTransaction txn) throws Exception { + return txn.treeExists(tree); + } + }), "a read of a table that was never created finds no tree"); + + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.treeExists(tree); + } + }); + fail("a writeable transaction must not report a rejected query as an absent tree"); + } catch (InvalidQueryException expected) { + // the table of a writeable open is created by openTree(), so its absence is a fault + } + + // Every tree of this backend is a partition of the one table, so it has no existence of + // its own: it is there once it holds a record, and not before. + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + assertFalse(txn.treeExists(tree), "an empty partition holds no record"); + txn.put(tree, key(0), value(0)); + assertTrue(txn.treeExists(tree)); + txn.deleteTree(tree); + assertFalse(txn.treeExists(tree)); + } + }); + } finally { + dropTree(storage, tree); + storage.close(); + } + } + private static void dropTree(CASStorage storage, final TreeName tree) { try { storage.write(new WriteOperation() { 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 22e8ea1bb9..cd8057efef 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 @@ -236,6 +236,211 @@ public void run(WriteableTransaction txn) throws Exception { } } + /** + * treeExists() has to answer for a table that was never created rather than fail: it is how the + * compressed schema tells a backend with nothing to migrate from one whose definitions are still + * under the shared prefix (#873), and every other statement of this storage fails outright on a + * table that does not exist. + */ + @Test + public void testTreeExistsAnswersForAMissingTable() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName present = new TreeName("testTreeExists", "present"); + final TreeName absent = new TreeName("testTreeExists", "absent"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(present, true); + assertTrue(txn.treeExists(present)); + assertFalse(txn.treeExists(absent)); + } + }); + // the read path has to answer as well: export-ldif and verify-index open read-only, where + // no tree is created and the question cannot be settled by writing one + storage.read(new ReadOperation() { + @Override + public Void run(ReadableTransaction txn) throws Exception { + assertTrue(txn.treeExists(present)); + assertFalse(txn.treeExists(absent)); + return null; + } + }); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(present); + assertFalse(txn.treeExists(present)); + } + }); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(present); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * The compressed schema definitions of this backend must live in a table of its own. The tree + * name they used to carry held no backend qualifier, so its table name was a constant that every + * JDBC backend of every server sharing the database mapped to, and two of them overwrote each + * other's token definitions there (#873). The backend of this suite has been opened and populated + * by PluggableBackendImplTestCase#setUp, so its own table exists by now. + */ + @Test + public void testCompressedSchemaTableIsQualifiedByBackendId() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + try { + storage.open(AccessMode.READ_WRITE); + final String shared = JDBCStorage.toTableName(new TreeName("compressed_schema", "compressed_attributes")); + final String own = JDBCStorage.toTableName( + new TreeName("compressed_schema_" + getBackendId(), "compressed_attributes")); + assertFalse(shared.equals(own), "the qualified tree name must map to a table of its own"); + try (final Connection con = DriverManager.getConnection(getJdbcUrl())) { + assertTrue(isExistingTable(con, own), own + " (this backend's own definitions) is missing"); + assertFalse(isExistingTable(con, shared), shared + " is the table every backend used to share"); + } + } finally { + storage.close(); + } + } + + /** + * Reading a tree must not enrol it in the storage's tree map: removeStorageFiles() drops every + * table that map names, and the compressed schema reads the tree its definitions used to be + * shared under - which on a shared database is another backend's to keep (#873). Asking whether + * the tree is there is only the first of those reads: the migration counts it and copies it out + * too, so one guarded statement would not be enough. + */ + @Test + public void testProbingATreeDoesNotPutItUpForRemoval() throws Exception { + final TreeName foreign = new TreeName("testProbe", "foreign"); + final JDBCStorage owner = new JDBCStorage(createBackendCfg(), null); + owner.open(AccessMode.READ_WRITE); + owner.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(foreign, true); + txn.put(foreign, key(1), value(1)); + } + }); + owner.close(); + + // a second storage on the same database, which never opened that tree - the shape of two + // backends addressing one database + final JDBCStorage other = new JDBCStorage(createBackendCfg(), null); + try { + other.open(AccessMode.READ_WRITE); + other.read(new ReadOperation() { + @Override + public Void run(ReadableTransaction txn) throws Exception { + // every read the compressed schema runs against a tree it does not own: it asks + // whether the tree is there, counts it, reads a key of it and walks it (#873) + assertTrue(txn.treeExists(foreign)); + assertEquals(txn.getRecordCount(foreign), 1); + assertEquals(txn.read(foreign, key(1)), value(1)); + try (final Cursor cursor = txn.openCursor(foreign)) { + assertTrue(cursor.next()); + assertEquals(cursor.getKey(), key(1)); + } + return null; + } + }); + assertFalse(other.listTrees().contains(foreign), "a tree only read must not be listed for removal"); + + other.removeStorageFiles(); + + try (final Connection con = DriverManager.getConnection(getJdbcUrl())) { + assertTrue(isExistingTable(con, JDBCStorage.toTableName(foreign)), + "clearing one backend dropped a table it had only asked about"); + } + } finally { + other.close(); + final JDBCStorage cleanup = new JDBCStorage(createBackendCfg(), null); + try { + cleanup.open(AccessMode.READ_WRITE); + cleanup.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(foreign); + } + }); + } catch (Exception ignored) { + } finally { + cleanup.close(); + } + } + } + + /** + * The other side of the same rule: a cursor reads through the non-enrolling name, but deleting + * through one writes to the tree, so it is a tree this backend owns and removeStorageFiles() + * has to be able to name it. + */ + @Test + public void testDeletingThroughACursorPutsTheTreeUpForRemoval() throws Exception { + final TreeName tree = new TreeName("testCursorDelete", "tree"); + final JDBCStorage owner = new JDBCStorage(createBackendCfg(), null); + owner.open(AccessMode.READ_WRITE); + owner.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + txn.put(tree, key(1), value(1)); + } + }); + owner.close(); + + // a storage that never opened that tree, so nothing but the delete can enrol it + final JDBCStorage other = new JDBCStorage(createBackendCfg(), null); + try { + other.open(AccessMode.READ_WRITE); + other.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + try (final Cursor cursor = txn.openCursor(tree)) { + assertTrue(cursor.next()); + cursor.delete(); + } + } + }); + assertTrue(other.listTrees().contains(tree), "a tree written through a cursor must be listed for removal"); + } finally { + other.close(); + final JDBCStorage cleanup = new JDBCStorage(createBackendCfg(), null); + try { + cleanup.open(AccessMode.READ_WRITE); + cleanup.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) { + } finally { + cleanup.close(); + } + } + } + + private static boolean isExistingTable(Connection con, String tableName) throws SQLException { + try (final ResultSet rs = con.getMetaData().getTables(null, null, null, new String[]{"TABLE"})) { + while (rs.next()) { + if (tableName.equalsIgnoreCase(rs.getString("TABLE_NAME"))) { + return true; + } + } + } + return false; + } + /** * Forward repositioning inside the already-fetched batch must be served from the buffer without SQL, * and batch sizes must grow from "fetchsize.initial" to "fetchsize" on sequential reads (#860). 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 e3b01b86d8..00cbcec6e3 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 @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2015-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.pluggable; @@ -125,7 +126,8 @@ private static DefaultIndex newIndex(String name, int indexLimit, EnumSet> storage = new HashMap<>(); @@ -248,10 +250,21 @@ public long getRecordCount(TreeName treeName) return getTree(treeName).size(); } + @Override + public boolean treeExists(TreeName treeName) + { + return storage.containsKey(treeName); + } + @Override public void openTree(TreeName name, boolean createOnDemand) { - storage.put(name, new TreeMap()); + // Honours createOnDemand, and leaves an already open tree alone rather than replacing it with + // an empty one, so that a caller opening the same tree twice does not silently lose its records. + if (createOnDemand) + { + storage.putIfAbsent(name, new TreeMap()); + } } @Override diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PersistentCompressedSchemaTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PersistentCompressedSchemaTest.java new file mode 100644 index 0000000000..f2b98de18a --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/PersistentCompressedSchemaTest.java @@ -0,0 +1,337 @@ +/* + * 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.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.testng.Assert.fail; + +import org.forgerock.opendj.ldap.ByteSequence; +import org.forgerock.opendj.ldap.ByteString; +import org.forgerock.opendj.ldap.ByteStringBuilder; +import org.opends.server.DirectoryServerTestCase; +import org.opends.server.TestCaseUtils; +import org.opends.server.backends.pluggable.DefaultIndexTest.DummyWriteableTransaction; +import org.opends.server.backends.pluggable.spi.AccessMode; +import org.opends.server.backends.pluggable.spi.Cursor; +import org.opends.server.backends.pluggable.spi.Storage; +import org.opends.server.backends.pluggable.spi.StorageRuntimeException; +import org.opends.server.backends.pluggable.spi.TreeName; +import org.opends.server.backends.pluggable.spi.WriteOperation; +import org.opends.server.core.ServerContext; +import org.opends.server.types.Attributes; +import org.opends.server.types.InitializationException; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +/** + * The compressed schema of a backend must not be shared with another backend that happens to + * address the same database, and a backend upgraded from a version that did share it must keep + * decoding the entries it wrote before the upgrade (issue #873). + */ +@SuppressWarnings("javadoc") +@Test(groups = { "precommit", "pluggablebackend" }, sequential = true) +public class PersistentCompressedSchemaTest extends DirectoryServerTestCase +{ + private static final String AD = "compressed_attributes"; + private static final String OC = "compressed_object_classes"; + + /** The pair every backend wrote to before the definitions were separated. */ + private static final TreeName LEGACY_AD = new TreeName("compressed_schema", AD); + private static final TreeName LEGACY_OC = new TreeName("compressed_schema", OC); + + private ServerContext serverContext; + private SharedDatabase txn; + private Storage storage; + + @BeforeClass + public void startServer() throws Exception + { + // Needed for the schema: loading a definition resolves its attribute type against the server's. + TestCaseUtils.startServer(); + serverContext = TestCaseUtils.getServerContext(); + } + + @BeforeMethod + public void setUp() throws Exception + { + // One transaction shared by every backend of a test stands for one database addressed by all of + // them - the JDBC deployment of the issue. On JE or PDB each backend would have its own. + txn = new SharedDatabase(); + storage = mock(Storage.class); + doAnswer(invocation -> { + ((WriteOperation) invocation.getArguments()[0]).run(txn); + return null; + }).when(storage).write(any(WriteOperation.class)); + } + + /** + * The heart of the issue: both backends allocate the first token of their own map for a different + * attribute description, so if they shared a tree the second would overwrite the definition of + * the first and the entries of the first would decode as the wrong attribute after a restart. + */ + @Test + public void backendsSharingOneDatabaseDoNotShareTheTokenSpace() throws Exception + { + // Both are opened before either writes, as they are at server start: each loads the same state + // and so holds the same next token. Opening the second one after the first had written would + // hide the issue, since it would have loaded the definition the first one just stored. + final PersistentCompressedSchema backendA = open("backendA", AccessMode.READ_WRITE); + final PersistentCompressedSchema backendB = open("backendB", AccessMode.READ_WRITE); + final ByteString encodedByA = encode(backendA, "cn"); + final ByteString encodedByB = encode(backendB, "sn"); + + // Re-opening is what a restart does: the definitions come back from the trees, not from memory. + assertThat(decode(open("backendA", AccessMode.READ_WRITE), encodedByA)).isEqualTo("cn"); + assertThat(decode(open("backendB", AccessMode.READ_WRITE), encodedByB)).isEqualTo("sn"); + + // and nothing was written under the prefix they used to share + assertThat(txn.treeExists(LEGACY_AD)).isFalse(); + assertThat(txn.treeExists(LEGACY_OC)).isFalse(); + } + + /** A backend upgraded from a version that wrote under the shared prefix finds its definitions. */ + @Test + public void definitionsWrittenBeforeTheUpgradeAreMigrated() throws Exception + { + final ByteString encoded = encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + + assertThat(decode(open("backendA", AccessMode.READ_WRITE), encoded)).isEqualTo("cn"); + assertThat(txn.getRecordCount(ownTree("backendA", AD))).isEqualTo(txn.getRecordCount(LEGACY_AD)); + assertThat(txn.getRecordCount(ownTree("backendA", OC))).isEqualTo(txn.getRecordCount(LEGACY_OC)); + } + + /** + * The shared trees are read, never emptied: on a database addressed by several backends they may + * still be the only copy another backend has, and leaving them is what makes a downgrade possible. + */ + @Test + public void theSharedTreesSurviveTheMigration() throws Exception + { + encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + final long legacyRecords = txn.getRecordCount(LEGACY_AD); + + open("backendA", AccessMode.READ_WRITE); + + assertThat(txn.getRecordCount(LEGACY_AD)).isEqualTo(legacyRecords); + } + + /** + * A migration interrupted halfway - a storage without transactions can stop between two records - + * is finished by the next open rather than leaving the backend with a partial token space. + */ + @Test + public void anInterruptedMigrationIsFinishedByTheNextOpen() throws Exception + { + final ByteString encoded = encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + open("backendA", AccessMode.READ_WRITE); + + // undo one record of the completed migration, which is what an interrupted one would have left + txn.delete(ownTree("backendA", AD), firstKeyOf(ownTree("backendA", AD))); + + assertThat(decode(open("backendA", AccessMode.READ_WRITE), encoded)).isEqualTo("cn"); + assertThat(txn.getRecordCount(ownTree("backendA", AD))).isEqualTo(txn.getRecordCount(LEGACY_AD)); + } + + /** + * export-ldif and verify-index open the root container read-only, where the migration cannot run. + * The definitions must still be found, and nothing may be written - not even the empty trees an + * openTree() would leave behind, which is why {@link SharedDatabase} creates one whenever it is + * asked for a tree at all. + */ + @Test + public void aReadOnlyOpenReadsTheSharedTreesWhereTheyLie() throws Exception + { + final ByteString encoded = encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + + assertThat(decode(open("backendA", AccessMode.READ_ONLY), encoded)).isEqualTo("cn"); + assertThat(txn.treeExists(ownTree("backendA", AD))).isFalse(); + assertThat(txn.treeExists(ownTree("backendA", OC))).isFalse(); + } + + /** + * The migration fills the gaps of this backend and touches nothing else: where the shared trees + * and this backend disagree about what a token stands for - which is the state issue #873 leaves + * behind, two backends having allocated the same token for different attributes - the definition + * this backend's own entries were encoded against is the one that survives. + */ + @Test + public void aDefinitionOfThisBackendIsNeverOverwrittenByTheSharedOne() throws Exception + { + final ByteString encodedByA = encode(open("backendA", AccessMode.READ_WRITE), "cn"); + + // What another backend left under the shared prefix before the upgrade: the same first token, + // standing for a different attribute, and one definition beyond what backendA holds, so that + // the record counts send the migration on its way. + final PersistentCompressedSchema backendB = open("backendB", AccessMode.READ_WRITE); + encode(backendB, "sn"); + encode(backendB, "description"); + copyTree(ownTree("backendB", AD), LEGACY_AD); + copyTree(ownTree("backendB", OC), LEGACY_OC); + + assertThat(decode(open("backendA", AccessMode.READ_WRITE), encodedByA)).isEqualTo("cn"); + // and the definition backendA did not have was still copied, so the migration did run + assertThat(txn.getRecordCount(ownTree("backendA", AD))).isEqualTo(txn.getRecordCount(LEGACY_AD)); + } + + /** + * A migration that cannot complete is fatal to the open: carrying on with the definitions half + * copied would restart the token allocation part way through the map the entries were encoded + * with, and report nothing. + */ + @Test + public void aMigrationThatCannotCompleteFailsTheOpen() throws Exception + { + encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + final RuntimeException failure = new IllegalStateException("no space left on device"); + txn.failEveryWriteWith(failure); + + try + { + open("backendA", AccessMode.READ_WRITE); + fail("the open must not succeed on a migration that failed"); + } + catch (final InitializationException e) + { + assertThat(e.getCause()).isSameAs(failure); + // and the message names the backend and both trees, since which of the two failed is the point + assertThat(e.getMessage()).contains("backendA") + .contains(LEGACY_AD.toString()).contains(ownTree("backendA", AD).toString()); + } + } + + /** + * A failure the storage reports as its own is left with the type it was given. The migration runs + * inside the WriteOperation of {@link RootContainer#open}, and PDBStorage.write() decides by type + * what to do with what leaves it: a transaction conflict it would have replayed must not come out + * as something else, or the open fails where it used to be retried. + */ + @Test + public void aStorageFailureIsLeftForTheStorageToRecognize() throws Exception + { + encode(open("backendA", AccessMode.READ_WRITE), "cn"); + makeStoreLookPreUpgrade("backendA"); + final StorageRuntimeException conflict = new StorageRuntimeException("transaction rolled back"); + txn.failEveryWriteWith(conflict); + + try + { + open("backendA", AccessMode.READ_WRITE); + fail("the open must not succeed on a migration that failed"); + } + catch (final StorageRuntimeException e) + { + assertThat(e).isSameAs(conflict); + } + } + + private PersistentCompressedSchema open(String backendId, AccessMode accessMode) throws Exception + { + return new PersistentCompressedSchema(serverContext, backendId, storage, txn, accessMode); + } + + private static TreeName ownTree(String backendId, String indexId) + { + return new TreeName("compressed_schema_" + backendId, indexId); + } + + private ByteString encode(PersistentCompressedSchema schema, String attributeName) throws Exception + { + final ByteStringBuilder builder = new ByteStringBuilder(); + schema.encodeAttribute(builder, Attributes.create(attributeName, "a value")); + return builder.toByteString(); + } + + private String decode(PersistentCompressedSchema schema, ByteString encoded) throws Exception + { + return schema.decodeAttribute(encoded.asReader()).getAttributeDescription().getAttributeType().getNameOrOID(); + } + + /** Moves the definitions of a backend back under the shared prefix, as an earlier version left them. */ + private void makeStoreLookPreUpgrade(String backendId) throws Exception + { + copyTree(ownTree(backendId, AD), LEGACY_AD); + copyTree(ownTree(backendId, OC), LEGACY_OC); + txn.deleteTree(ownTree(backendId, AD)); + txn.deleteTree(ownTree(backendId, OC)); + } + + private void copyTree(TreeName from, TreeName to) + { + txn.openTree(to, true); + try (Cursor cursor = txn.openCursor(from)) + { + while (cursor.next()) + { + txn.put(to, cursor.getKey(), cursor.getValue()); + } + } + } + + private ByteString firstKeyOf(TreeName treeName) + { + try (Cursor cursor = txn.openCursor(treeName)) + { + assertThat(cursor.next()).isTrue(); + return cursor.getKey(); + } + } + + /** + * The one database every backend of a test addresses. Two behaviours of a real storage are + * modelled on purpose, so that the assertions above can fail: + *

+ */ + private static final class SharedDatabase extends DummyWriteableTransaction + { + private RuntimeException failure; + + void failEveryWriteWith(RuntimeException failure) + { + this.failure = failure; + } + + @Override + public void openTree(TreeName name, boolean createOnDemand) + { + super.openTree(name, true); + } + + @Override + public void put(TreeName treeName, ByteSequence key, ByteSequence value) + { + if (failure != null) + { + throw failure; + } + super.put(treeName, key, value); + } + } +}