From add86d3c7c047215f886c51e2b29a4c7f1e86c0c Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Thu, 03 Sep 2026 08:21:14 +0000
Subject: [PATCH] [#877] Bound a statement of the JDBC backend by the class of the work it belongs to (#882)
---
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java | 15
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java | 49
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/PersistentCompressedSchema.java | 7
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java | 1170 +++++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java | 50
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Index.java | 18
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/BulkCursorTest.java | 365 +++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 922 ++++++++++++++++--
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendStat.java | 6
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ShardedCounter.java | 55
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/TracedStorage.java | 18
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2Entry.java | 14
opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2EntryTest.java | 55 +
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/EntryContainer.java | 24
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 194 +++
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/ReadableTransaction.java | 20
opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ExportJob.java | 11
17 files changed, 2,855 insertions(+), 138 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 f1b9419..9f43728 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
@@ -41,6 +41,8 @@
import java.sql.*;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Predicate;
import static org.opends.server.backends.pluggable.spi.StorageUtils.addErrorMessage;
@@ -142,21 +144,548 @@
return ccr;
}
- ResultSet executeResultSet(PreparedStatement statement) throws SQLException {
+ /**
+ * What a statement of this backend may legitimately take, and the property bounding it. One
+ * value cannot serve both: an entry read is a single row of an index, while the count of a
+ * tree and the delete that empties one before an import are a scan and a rewrite of a whole
+ * table, which take minutes on a populated backend and are not a symptom of anything.
+ */
+ enum StatementBound {
+ /** one row by primary key, or one batch of a cursor along its index */
+ OPERATION("org.openidentityplatform.opendj.jdbc.query.timeout", 120),
+ /**
+ * a whole table at once: count(*), the delete of clearTree, the scan behind the highest
+ * entry id, create index, drop table, and every batch of a cursor walking a tree whole.
+ * This class ships <em>unbounded</em>: what such a statement legitimately takes follows the
+ * size of the backend and the speed of its database, neither of which can be guessed here,
+ * so the deployment that knows both sets the property - until it does, a create index
+ * waiting for a metadata lock still waits for as long as the engine lets it, and so do the
+ * walks a backend makes while it opens (the load of the compressed schema, the read that
+ * checks id2entry is there) and the export behind the generation ID of a replicated domain.
+ * That is what this backend did before any of these bounds existed; bounding them as the
+ * work of a client operation, which is the only other value there was to give them, stopped
+ * a large backend from opening at all.
+ */
+ BULK("org.openidentityplatform.opendj.jdbc.bulk.timeout", 0);
+
+ final String property;
+ final int defaultSeconds;
+
+ StatementBound(String property, int defaultSeconds) {
+ this.property = property;
+ this.defaultSeconds = defaultSeconds;
+ }
+
+ /**
+ * The bound in seconds, as configured by {@link #property}: 0, or a negative value, leaves
+ * the statement unbounded, as it was before this bound existed, while a value that is not a
+ * number is ignored in favour of {@link #defaultSeconds} - {@code Integer.getInteger()}
+ * falls back to its default rather than reading such a value as a zero. A value above
+ * {@link JDBCStorage#MAX_BOUND_SECONDS} is taken down to it, for the reason recorded there.
+ */
+ int seconds() {
+ return clampSeconds(Integer.getInteger(property, defaultSeconds));
+ }
+ }
+
+ /** What a caller of {@link #executeResultSet} makes of the rows, while the bound is still armed. */
+ interface RowsHandler<T> {
+ T handle(ResultSet rows) throws SQLException;
+ }
+
+ /**
+ * The value of a row that is there. A row whose {@code v} is null is one this backend never
+ * wrote - the column is nullable, however it is written - and it must not be answered with the
+ * {@code null} a single-row read uses, which is already taken and means "no such key": read that
+ * way, a key that exists is reported as absent. Named rather than left to the bare
+ * {@code NullPointerException} of {@code ByteString.wrap}, which names neither the fault nor the
+ * table it is in, and a {@code RuntimeException} rather than an {@code SQLException}, so that a
+ * corrupt row is never weighed against the bound of the statement that read it and reported as a
+ * timeout of a property that would have changed nothing. The key is left out of the message for
+ * the reason {@link #timedOut} leaves the statement out of its own: it is entry data.
+ */
+ static ByteString valueOfRow(ResultSet rows, String tableName) throws SQLException {
+ return ByteString.wrap(valueOfRow(rows.getBytes("v"), tableName));
+ }
+
+ /**
+ * The same check where a batch of a cursor reads the value beside its key, by position. Checked
+ * as the rows are taken off the statement rather than as they are handed out one by one: there
+ * the failure is inside the bound and inside the {@code catch} of the batch, while a batch
+ * buffered whole and unwrapped later fails from {@code advanceFromBuffer()} - outside both, and
+ * as the bare {@code NullPointerException} this exists to replace.
+ */
+ static byte[] valueOfRow(byte[] value, String tableName) {
+ if (value == null) {
+ throw new StorageRuntimeException("jdbc: a row of "+tableName+" is present with no value");
+ }
+ return value;
+ }
+
+ <T> T executeResultSet(PreparedStatement statement, RowsHandler<T> rows) throws SQLException {
+ return executeResultSet(statement, StatementBound.OPERATION, rows);
+ }
+
+ /**
+ * Runs a query under the bound of its class and hands the rows to {@code rows} while that bound
+ * is still armed. They are read there rather than after this method returns because a driver
+ * transfers them as they are asked for: read outside, the transfer - up to a whole batch of a
+ * cursor - would run with neither layer of the bound covering it, which is exactly where a
+ * database that stops answering mid-drain parks the worker thread. {@code setQueryTimeout}
+ * covering {@code ResultSet.next()} is optional in the JDBC contract ("drivers <em>may</em>
+ * also apply this limit"), and the two drivers of this backend that do not buffer a result
+ * whole - oracle prefetches ten rows at a time, mssql buffers adaptively - are the ones that
+ * do not.
+ */
+ <T> T executeResultSet(PreparedStatement statement, StatementBound bound, RowsHandler<T> rows) throws SQLException {
if (logger.isTraceEnabled()) {
logger.trace(LocalizableMessage.raw("jdbc: %s",statement));
}
- return statement.executeQuery();
+ return bounded(statement, bound, () -> {
+ try (final ResultSet rs=statement.executeQuery()) {
+ return rows.handle(rs);
+ }
+ });
}
int execute(PreparedStatement statement) throws SQLException {
+ return execute(statement, StatementBound.OPERATION);
+ }
+
+ int execute(PreparedStatement statement, StatementBound bound) throws SQLException {
if (logger.isTraceEnabled()) {
logger.trace(LocalizableMessage.raw("jdbc: %s",statement));
}
- return statement.executeUpdate();
+ return bounded(statement, bound, statement::executeUpdate);
}
- // unlike execute(), tolerates statements that return a result set ("analyze table" on mysql)
+ interface Execution<T> {
+ T run() throws SQLException;
+ }
+
+ /**
+ * Runs a statement under the bound of its class. A statement of a class that carries one has to
+ * end: a row locked by an unrelated session, a table waiting for a metadata lock or a database
+ * that stops answering mid-query would otherwise park the worker thread that issued it for
+ * good. A class configured with no bound - which {@link StatementBound#BULK} ships as - takes
+ * neither of the two layers below and waits as this backend waited before they existed.
+ * <p>
+ * The bound is asked of the driver rather than of the session, because a pooled connection
+ * cannot carry a session setting - {@code CachedConnection.close()} only rolls back, so a
+ * {@code statement_timeout} of one operation would apply to whoever borrows the connection
+ * next - and it is applied in two layers, since the first one is not answered everywhere:
+ * {@code setQueryTimeout} cancels the statement and keeps the connection, while the socket read
+ * timeout behind it ends the wait even when the cancel is not acted upon. Oracle needs that
+ * second layer: a session blocked in a row-lock enqueue does not process the break its driver
+ * sends, so the timeout is armed and never arrives (the container suites cover it). That second
+ * layer belongs to the connection rather than to the statement, so it is arbitrated between the
+ * statements running on one - see {@link Backstop}.
+ */
+ private <T> T bounded(PreparedStatement statement, StatementBound bound, Execution<T> execution) throws SQLException {
+ final int seconds=bound.seconds();
+ // whether the cancel is in force: a driver is free to refuse the query timeout, and then the
+ // socket read timeout behind it is the only layer this statement has - one that arrives later
+ final boolean cancelArmed=seconds > 0 && setQueryTimeout(statement, seconds);
+ // an unbounded class is announced to the connection all the same: a statement told it may
+ // take as long as it needs must not be cut by the socket read timeout of a concurrent one
+ return bounded(connectionOf(statement), bound.property, seconds, cancelArmed, execution);
+ }
+
+ /**
+ * Runs the catalog lookups of {@code openTree()} under the bound of their class. They ask
+ * {@code DatabaseMetaData}, which takes no query timeout, so the socket read timeout behind the
+ * cancel is the only layer they can be given - and they do need one: they run once per tree on
+ * every open of a backend, and the catalog is answered by the same engine, behind the same
+ * locks, as the {@code create table} they guard.
+ * <p>
+ * That layer is only as good as what it actually arms, which is not always something: a driver
+ * with no network timeout, a connection that failed the call, one already carrying a tighter
+ * timeout of a deployment's own, and a statement of an unbounded class running beside this one
+ * each leave such a lookup with no bound at all. It is then reported as what it is - see
+ * {@link #timedOut} - rather than as a property that bounded nothing.
+ */
+ <T> T bounded(Connection con, StatementBound bound, Execution<T> execution) throws SQLException {
+ // no cancel to arm: DatabaseMetaData takes no query timeout, so the socket read timeout behind
+ // it is the only layer these have, and nothing ends their wait before the margin of that layer
+ return bounded(con, bound.property, bound.seconds(), false, execution);
+ }
+
+ /**
+ * Runs a statement under a bound of its own rather than under the bound of a class, for the one
+ * statement that has a property of its own: the statistics refresh after an import, which
+ * legitimately takes as long as a scan of the table it describes.
+ */
+ private <T> T bounded(Connection con, String property, int seconds, boolean cancelArmed, Execution<T> execution)
+ throws SQLException {
+ final long startedAt=nanoTime();
+ final Backstop backstop=holdBackstop(con, seconds);
+ try {
+ return execution.run();
+ }catch (SQLException e) {
+ // what the second layer carries is read here rather than at the top: it is arbitrated
+ // between the statements in flight, so it is the value at the moment of the failure that
+ // bounded this statement - and it is read before the release below takes it back off
+ throw timedOut(e, property, seconds, cancelArmed, armedMillis(backstop), startedAt);
+ }finally {
+ releaseBackstop(backstop, con, seconds);
+ }
+ }
+
+ /**
+ * What the socket read timeout of a connection carries for the statements on it right now, or 0
+ * where this layer is not in force for them at all. It is not enough that a bound was asked for:
+ * {@link #applyBackstop} arms nothing on a connection whose driver refused the call or has no
+ * network timeout to give, nothing on one already carrying a timeout of a deployment's own that
+ * is tighter than ours, and nothing while a statement of an unbounded class runs beside this one.
+ */
+ private static int armedMillis(Backstop state) {
+ if (state == null) {
+ return 0; // no connection to arm it on: the cancel is the whole bound of such a statement
+ }
+ synchronized (state) {
+ return state.armed;
+ }
+ }
+
+ /**
+ * Asks the driver to cancel the statement at the bound. Not every driver has one: the JDBC
+ * contract allows {@code SQLFeatureNotSupportedException} and this backend takes whatever URL a
+ * deployment configures, so a driver without it degrades to the socket read timeout behind it
+ * rather than failing every statement it is given.
+ */
+ private boolean setQueryTimeout(PreparedStatement statement, int seconds) {
+ try {
+ statement.setQueryTimeout(seconds);
+ return true;
+ }catch (SQLException | RuntimeException e) {
+ if (queryTimeoutWarned.compareAndSet(false, true)) {
+ logger.warn(LocalizableMessage.raw("jdbc: the driver would not take a query timeout (%s): a statement of this"
+ + " backend is left to the socket read timeout behind it", e.getMessage()));
+ }
+ return false;
+ }
+ }
+
+ private Connection connectionOf(PreparedStatement statement) {
+ try {
+ return statement.getConnection();
+ }catch (SQLException | RuntimeException e) {
+ return null; // nothing to arm the backstop on; the cancel above is the whole bound
+ }
+ }
+
+ /** How long the socket read timeout outlasts the cancel it backs up, giving it room to arrive. */
+ static final int BACKSTOP_MARGIN_SECONDS = 30;
+
+ /** How far under its bound a driver may report the cancel, its timer being kept in whole seconds. */
+ static final long CLOCK_SLACK_MILLIS = 250;
+
+ /**
+ * Ceiling of every bound this backend arms, in seconds - 24.9 days, which is what a socket read
+ * timeout can hold at all: {@code setNetworkTimeout} takes milliseconds of an {@code int}, and a
+ * bound past this one has no value of that layer to be given. It is <em>not</em> what keeps the
+ * arithmetic of {@link #backstopMillis} in range - the {@code long} multiply under the
+ * {@code Math.min} there does that on its own, up to the point where adding the margin overflows
+ * an {@code int} before the multiply ever runs - so a reader who later takes that {@code Math.min}
+ * away must not read this clamp as covering them.
+ * <p>
+ * Clamped rather than refused, and clamped rather than read as "no bound": a bound this large
+ * cancels nothing a database will not have ended first, so taking a nonsensical value down to it
+ * costs a deployment nothing, while reading it as an unbound would take a bound away from a
+ * deployment that asked for one. A property set to {@code Integer.MAX_VALUE} therefore bounds a
+ * statement at 24.9 days rather than leaving it unbounded; {@code 0} is what leaves it unbounded.
+ */
+ static final int MAX_BOUND_SECONDS = Integer.MAX_VALUE/1000 - BACKSTOP_MARGIN_SECONDS;
+
+ static int clampSeconds(int seconds) {
+ return Math.max(0, Math.min(MAX_BOUND_SECONDS, seconds));
+ }
+
+ /**
+ * What {@link #timedOut} calls the second layer when that layer is the only one a statement ran
+ * under, so that a test can tell the two apart in a message: a run where the first layer stopped
+ * working degrades to this one by design, silently, and a suite that only measures how long a
+ * statement waited would go green with the cancel gone entirely.
+ */
+ static final String BACKSTOP_ALONE = "the socket read timeout behind ";
+
+ // The clock a bound is measured on, in one place so that a test can drive it: the classification
+ // below turns on a few milliseconds either side of the bound, and a mock statement cannot be made
+ // to take a real second without the suite taking one too. Monotonic, so that a step of the wall
+ // clock can neither lengthen nor shorten what a statement is measured to have taken.
+ long nanoTime() {
+ return System.nanoTime();
+ }
+
+ // setNetworkTimeout() takes the executor its timeout handling runs on; the drivers of this
+ // backend only set a socket option in it, so it costs a call rather than a thread.
+ private static final Executor DIRECT_EXECUTOR = Runnable::run;
+
+ // Set when the driver of this storage has no network timeout to give at all, which is a property
+ // of the driver rather than of a connection: asking it again would cost a throw per statement,
+ // and the entry a connection's Backstop lives in is gone as soon as nothing runs on it. Held per
+ // storage rather than per JVM, like the warnings below: a driver that will not take one of these
+ // says so once for every backend running on it, instead of one backend silencing it for all.
+ private final AtomicBoolean backstopUnsupported = new AtomicBoolean();
+ private final AtomicBoolean backstopUnsupportedWarned = new AtomicBoolean();
+ private final AtomicBoolean backstopFailedWarned = new AtomicBoolean();
+ private final AtomicBoolean queryTimeoutWarned = new AtomicBoolean();
+
+ /**
+ * The socket read timeout of one connection, and the statements running on it. This second
+ * layer of the bound is a property of the socket rather than of a statement, so it cannot be
+ * armed and put back per statement wherever a connection carries more than one at a time: an
+ * {@code ImporterImpl} holds a single connection for the whole of an import and writes to it
+ * from every phase-one worker and every phase-two task, and there the first statement to finish
+ * would take the backstop away from every statement still in flight - while a statement whose
+ * class carries no bound at all would run under whatever value a concurrent one happened to
+ * arm, dying at it with nothing to say which property cut it, since such a statement never
+ * reaches {@link #timedOut}.
+ * <p>
+ * So the value armed is the loosest of the bounds of the statements in flight, and a statement
+ * with no bound of its own takes it off for as long as it runs: this backstop exists to end a
+ * wait nothing else would end, never to cut a statement that was told it may take as long as it
+ * needs. What the connection carried before is put back when the last of them is through.
+ */
+ private static final class Backstop {
+ /** Bounds of the statements in flight, in milliseconds and by count, the loosest last. */
+ final TreeMap<Integer,Integer> bounds=new TreeMap<>();
+ /** Statements in flight with no bound of their own, which no backstop may cut short. */
+ int unbounded;
+ /** Statements holding this entry, bounded or not: at zero it leaves {@link #backstops}. */
+ int holders;
+ /** What the connection carried before the backstop armed it, and is given back afterwards. */
+ int previous;
+ /** What the backstop has armed, or 0 when the connection carries {@link #previous}. */
+ int armed;
+ /**
+ * Set when the driver would not take a network timeout on this connection: it is not asked
+ * again while the statements holding this entry run. A connection is the right scope for
+ * that: the common cause is a connection on its way out, and a driver that has no network
+ * timeout at all is remembered for the whole storage instead - see {@link #backstopUnsupported}.
+ */
+ boolean failed;
+ }
+
+ // Keyed by identity on the connection of the driver: CachedConnection.prepareStatement() hands
+ // the statement to the connection it wraps, so that is the one a statement reports, while the
+ // catalog lookups above hold the wrapper of that same connection - both have to find the same
+ // entry, so a wrapper is unwrapped on the way in. Static because the pool these connections
+ // come from is static; an entry lives only while statements are running on its connection.
+ private static final Map<Connection,Backstop> backstops = new IdentityHashMap<>();
+
+ private static Connection physical(Connection con) {
+ return con instanceof CachedConnection ? ((CachedConnection)con).parent : con;
+ }
+
+ /**
+ * Puts the bound of a statement about to run on the connection that will run it, and makes the
+ * socket read timeout of that connection fit every statement in flight on it. Reaching this
+ * bound, unlike reaching the cancel it backs up, costs the connection: the driver closes it,
+ * which is the price of a wait the database was never going to end on its own.
+ */
+ private Backstop holdBackstop(Connection con, int seconds) {
+ final Connection physical=physical(con);
+ if (physical == null) {
+ return null;
+ }
+ final Backstop state;
+ synchronized (backstops) {
+ state=backstops.computeIfAbsent(physical, c -> new Backstop());
+ state.holders++; // held from here, so that the entry outlives a concurrent release
+ }
+ synchronized (state) {
+ if (seconds > 0) {
+ state.bounds.merge(backstopMillis(seconds), 1, Integer::sum);
+ }else {
+ state.unbounded++;
+ }
+ applyBackstop(physical, state);
+ }
+ return state;
+ }
+
+ private void releaseBackstop(Backstop state, Connection con, int seconds) {
+ if (state == null) {
+ return;
+ }
+ final Connection physical=physical(con);
+ try {
+ synchronized (state) {
+ if (seconds > 0) {
+ final int millis=backstopMillis(seconds);
+ final Integer inFlight=state.bounds.get(millis);
+ if (inFlight == null || inFlight <= 1) {
+ state.bounds.remove(millis);
+ }else {
+ state.bounds.put(millis, inFlight-1);
+ }
+ }else {
+ state.unbounded--;
+ }
+ applyBackstop(physical, state);
+ }
+ }finally { // the entry is let go whatever the driver did, so that it cannot outlive its connection
+ synchronized (backstops) {
+ if (--state.holders <= 0) { // nothing is running on it: the connection is on its own again
+ backstops.remove(physical);
+ }
+ }
+ }
+ }
+
+ private static int backstopMillis(int seconds) {
+ return (int) Math.min(Integer.MAX_VALUE, (seconds+BACKSTOP_MARGIN_SECONDS)*1000L);
+ }
+
+ /**
+ * Makes the socket read timeout of the connection what the statements in flight on it need: the
+ * loosest of their bounds, or nothing of ours at all while one of them carries no bound. Called
+ * with the monitor of {@code state} held, since it both reads those counts and acts on the
+ * driver.
+ */
+ private void applyBackstop(Connection con, Backstop state) {
+ if (state.failed || backstopUnsupported.get()) {
+ // but a connection this backstop has already armed does not keep carrying it: the entry
+ // remembering what it carried before is dropped when its last statement is through, and the
+ // value would go back to the pool as the connection's own read timeout. Reachable through
+ // the second guard, which is a latch of the whole storage: a connection armed before it was
+ // set would otherwise never be disarmed. Where nothing was armed this costs no call.
+ restorePrevious(con, state);
+ return;
+ }
+ final int wanted=state.unbounded > 0 || state.bounds.isEmpty() ? 0 : state.bounds.lastKey();
+ try {
+ if (wanted == 0) {
+ if (state.armed != 0) {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, state.previous);
+ state.armed=0;
+ }
+ return;
+ }
+ if (state.armed == 0) {
+ state.previous=con.getNetworkTimeout();
+ }
+ // only ever tighten: a connection that already carries a read timeout carries one a
+ // deployment asked for, and this backstop exists to cap a cancel that is not acted
+ // upon, not to relax anything. 0 is "no timeout" in the JDBC contract, so it is the
+ // one value there is always something to gain by replacing.
+ if (state.previous > 0 && state.previous <= wanted) {
+ if (state.armed != 0) {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, state.previous);
+ state.armed=0;
+ }
+ return;
+ }
+ if (state.armed != wanted) {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, wanted);
+ state.armed=wanted;
+ }
+ }catch (SQLException | RuntimeException e) {
+ state.failed=true; // whatever the cause, this connection is not asked again while it runs
+ // and what it carried before goes back, while there is still an entry saying what that was:
+ // this one is dropped as soon as the last statement on the connection is through, and a
+ // backstop left armed would go back to the pool as the connection's own read timeout - which
+ // is exactly how the next borrower reads it, tightening to it and never replacing it.
+ restorePrevious(con, state);
+ // The two causes are told apart, because they deserve opposite treatment and one of them
+ // would otherwise spend the single warning the other needs: a driver with no network
+ // timeout at all says so through SQLFeatureNotSupportedException, and there is nothing to
+ // gain by asking it once per statement for the life of the storage, while a connection on
+ // its way out - it may be the one that reached this very timeout - says nothing about the
+ // driver and must not disable the backstop for the connections that are still healthy.
+ if (e instanceof SQLFeatureNotSupportedException) {
+ backstopUnsupported.set(true);
+ if (backstopUnsupportedWarned.compareAndSet(false, true)) {
+ logger.warn(LocalizableMessage.raw("jdbc: the driver takes no socket read timeout (%s): a statement the"
+ + " database does not cancel will wait for it indefinitely, unless the connect properties of the URL"
+ + " configured for this backend carry one", e.getMessage()));
+ }
+ }else if (backstopFailedWarned.compareAndSet(false, true)) {
+ logger.warn(LocalizableMessage.raw("jdbc: the socket read timeout backing up a cancelled statement could not"
+ + " be set on a connection (%s): a statement the database does not cancel will wait for it"
+ + " indefinitely there", e.getMessage()));
+ }
+ }
+ }
+
+ /**
+ * Gives the connection back the read timeout it carried before this backstop armed one, and
+ * forgets having armed it. Best effort by construction: the caller reaches this from a driver
+ * call that has just failed, so the connection may well be gone - and where it is, it is the
+ * driver that closes it rather than this backend.
+ */
+ private static void restorePrevious(Connection con, Backstop state) {
+ if (state.armed == 0) {
+ return; // the connection carries its own value already
+ }
+ try {
+ con.setNetworkTimeout(DIRECT_EXECUTOR, state.previous);
+ }catch (SQLException | RuntimeException ignored) {
+ // nothing further can be done for this connection here, and the failure to report is the
+ // one that brought us into the catch above
+ }finally {
+ state.armed=0;
+ }
+ }
+
+ // Every driver reports a cancelled statement differently - postgresql as 57014, oracle as
+ // ORA-01013, and neither of them as a SQLTimeoutException - so the bound is recognized by the
+ // time the statement took rather than by the class or the state of its failure, and that time
+ // is taken from the monotonic clock, which a step of the wall clock can neither lengthen nor
+ // shorten. What it cannot tell apart is a failure of another kind arriving after the bound,
+ // which is why the failure it replaces is chained rather than swallowed. The SQL state and the
+ // error number are carried over as well, since a failure that arrives at the bound may still be
+ // one a caller classifies: a mysql lock wait, reported in class 40, ends inside a longer bound
+ // and stays the replayable conflict it is. The statement itself is left out of the message: a
+ // driver renders it with its parameters bound, and those are entry data.
+ private SQLException timedOut(SQLException e, String property, int seconds, boolean cancelArmed,
+ int backstopArmedMillis, long startedAt) {
+ if (seconds <= 0) {
+ return e;
+ }
+ // Which layer was really in force, and until when. Where the cancel is armed, the property
+ // ends the wait at its own value. Where it is not - a statement of DatabaseMetaData takes no
+ // query timeout, and a driver is free to refuse one - the socket read timeout behind it is the
+ // only layer there is, and that one arrives a margin later: measuring such a statement against
+ // the property alone reported a connection reset at 121 s as a query timeout of 120 s and sent
+ // the operator to a property that bounded nothing. Asking for that layer is not having it,
+ // which is why the value armed is passed in rather than derived from the property here: a
+ // driver with no network timeout, a connection that failed the call, one already carrying a
+ // tighter timeout of its own, and a statement of an unbounded class running beside this one
+ // each leave it unarmed. A statement neither layer bounded reached no bound of ours at all, so
+ // its failure is the driver's own and is left exactly as it is: naming a property that armed
+ // nothing sends an operator to raise a value that changes nothing about the wait they saw.
+ final long endsAfterMillis=cancelArmed ? seconds*1000L : backstopArmedMillis;
+ if (endsAfterMillis <= 0) {
+ return e;
+ }
+ final long elapsedMillis=(nanoTime()-startedAt)/1000000L;
+ // The bound is allowed a little slack under it: a driver keeps its timer in whole seconds and
+ // reports the cancel a few milliseconds before the bound is arithmetically due, and measured
+ // to the millisecond such a statement would arrive as a bare 57014 or ORA-01013, naming
+ // neither the property that cancelled it nor the fact that it was cancelled at all.
+ if (elapsedMillis < endsAfterMillis-CLOCK_SLACK_MILLIS) {
+ return e;
+ }
+ // The time is reported as measured rather than as the bound. Where the database does not act
+ // on the cancel - a session blocked in a row-lock enqueue on oracle - the wait ends at the
+ // socket read timeout, a margin past the property that armed it, and "did not finish within
+ // the 120s" of a statement that waited 150 s is a message an operator cannot put next to a
+ // clock. The property named still governs both layers, since backstopMillis() derives the
+ // second one from it, so raising it stays the remedy either way.
+ return new SQLTimeoutException("jdbc: the statement took "+elapsedMillis+" ms, reaching the "
+ +(endsAfterMillis/1000L)+"s of "
+ +(cancelArmed ? property : BACKSTOP_ALONE+property+" (the only layer bounding a statement that takes no"
+ +" query timeout; it is armed at the loosest bound of the statements sharing this connection, this"
+ +" one's being "+seconds+"s plus the margin of that layer)")
+ +": raise that property, or set it to 0 for no bound", e.getSQLState(), e.getErrorCode(), e);
+ }
+
+ // Unlike execute(), tolerates a statement that returns a result set - the comment statement of
+ // mssql is a batch that ends in an exec - and, unlike it, carries no bound of its own: what is
+ // left of this method runs on a stamp connection, which is given a lock timeout of its own
+ // (Dialect.lockTimeoutSql) and a socket read timeout in its connect properties.
void executeAny(PreparedStatement statement) throws SQLException {
if (logger.isTraceEnabled()) {
logger.trace(LocalizableMessage.raw("jdbc: %s",statement));
@@ -165,7 +694,7 @@
}
Connection getConnection() throws Exception {
- return CachedConnection.getConnection(config.getDBDirectory());
+ return getConnection(true);
}
/**
@@ -177,7 +706,15 @@
* per import or per removal buys back exactly what master did on every borrow.
*/
Connection getValidatedConnection() throws Exception {
- return CachedConnection.getConnection(config.getDBDirectory(), false);
+ return getConnection(false);
+ }
+
+ // The one borrow of this storage: both methods above go through it, so that whatever stands in
+ // for the pool stands in for every path that takes a connection. A stand-in of the trusted
+ // borrow alone let the open, the import and the removal - the three that ask for a validated
+ // one - reach a real database instead.
+ Connection getConnection(boolean trusted) throws Exception {
+ return CachedConnection.getConnection(config.getDBDirectory(), trusted);
}
@@ -423,9 +960,8 @@
// Asked of the very session that parses the literal: sql_mode is a session setting, and a
// session opened at another moment can have been given another value of it.
boolean isMysqlBackslashEscape(Connection con) throws SQLException {
- try (final PreparedStatement statement=con.prepareStatement("select @@sql_mode");
- final ResultSet rs=executeResultSet(statement)) {
- final String sqlMode=rs.next() ? rs.getString(1) : null;
+ try (final PreparedStatement statement=con.prepareStatement("select @@sql_mode")) {
+ final String sqlMode=executeResultSet(statement, rs -> rs.next() ? rs.getString(1) : null);
return sqlMode==null || !sqlMode.toUpperCase().contains("NO_BACKSLASH_ESCAPES");
}
}
@@ -529,6 +1065,10 @@
// A session setting must reach the server as a plain batch: the sql server driver runs a
// prepared statement through sp_executesql, and a setting made there is reverted when that
// call returns - before the statement it is meant to protect ever runs.
+ //
+ // Outside both layers of the bound, like the comment statement executeAny() runs, and for the
+ // same reason: this is issued from newStampConnection() on a stamp connection, whose connect
+ // properties carry a socket read timeout of their own (Dialect.connectProperties).
private void executeSessionStatement(Connection con, String sql) throws SQLException {
try (final Statement statement=con.createStatement()) {
if (logger.isTraceEnabled()) {
@@ -732,9 +1272,7 @@
}
try (final PreparedStatement statement=con.prepareStatement(sql)) {
statement.setString(1,arg);
- try (final ResultSet rs=executeResultSet(statement)) {
- return rs.next() ? rs.getString(1) : null;
- }
+ return executeResultSet(statement, rs -> rs.next() ? rs.getString(1) : null);
}
}
@@ -763,7 +1301,7 @@
if (dialect==null) { // no portable statistics refresh for other engines
return false; // nothing was refreshed: reporting success here would make the assertion of the tests vacuous
}
- final int timeoutSeconds=Math.max(0,Integer.getInteger(STATISTICS_TIMEOUT_PROPERTY,STATISTICS_TIMEOUT_SECONDS_DEFAULT));
+ final int timeoutSeconds=clampSeconds(Integer.getInteger(STATISTICS_TIMEOUT_PROPERTY,STATISTICS_TIMEOUT_SECONDS_DEFAULT));
boolean allRefreshed=true;
for (final TreeName treeName : trees) {
final String tableName=getTableName(treeName);
@@ -795,21 +1333,36 @@
throw new IllegalStateException("no statistics refresh for dialect "+dialect);
}
try (final PreparedStatement statement=con.prepareStatement(sql)) {
- statement.setQueryTimeout(timeoutSeconds); // 0: wait without limit
+ // 0: wait without limit - and false where the driver would not take the cancel, which
+ // leaves the socket read timeout behind it as the only layer this refresh runs under
+ final boolean cancelArmed=timeoutSeconds>0 && setQueryTimeout(statement, timeoutSeconds);
for (int i=0;i<args.length;i++) {
statement.setString(i+1,args[i]);
}
- if (dialect==Dialect.MYSQL) { // mysql reports analyze problems as a result row, not an SQLException
- try (final ResultSet rs=executeResultSet(statement)) {
- while (rs.next()) {
- if ("error".equalsIgnoreCase(rs.getString("Msg_type"))) {
- throw new SQLException(rs.getString("Msg_text"));
+ // Under the bound of the statistics refresh rather than under a class of
+ // StatementBound, which would put its own value over one this statement has a
+ // property for - but under both layers of it all the same: on oracle this is
+ // dbms_stats.gather_table_stats, the engine whose session does not act on the
+ // break its driver sends, and it runs at the very end of a successful import,
+ // where a cancel that never arrives would park it with the data already
+ // committed and nothing left to report.
+ bounded(con, STATISTICS_TIMEOUT_PROPERTY, timeoutSeconds, cancelArmed, () -> {
+ if (logger.isTraceEnabled()) {
+ logger.trace(LocalizableMessage.raw("jdbc: %s",statement));
+ }
+ if (dialect==Dialect.MYSQL) { // mysql reports analyze problems as a result row, not an SQLException
+ try (final ResultSet rs=statement.executeQuery()) {
+ while (rs.next()) {
+ if ("error".equalsIgnoreCase(rs.getString("Msg_type"))) {
+ throw new SQLException(rs.getString("Msg_text"));
+ }
}
}
+ }else { // tolerates a statement that returns a result set, which execute() does not
+ statement.execute();
}
- }else {
- executeAny(statement);
- }
+ return null;
+ });
con.commit();
}
}catch (Exception e) {
@@ -840,7 +1393,7 @@
try {
for (final TreeName treeName : trees) {
try (final PreparedStatement statement = con.prepareStatement("drop table " + getTableName(treeName))) {
- execute(statement);
+ execute(statement, StatementBound.BULK);
}
}
con.commit();
@@ -1330,22 +1883,39 @@
return driverNameOf(con).contains("microsoft") ? "cast(? as char(128))" : "?";
}
- private class ReadableTransactionImpl implements ReadableTransaction {
+ class ReadableTransactionImpl implements ReadableTransaction {
final Connection con;
+ /**
+ * The class the statements of this transaction take. It follows who runs them rather than
+ * what they look like: an import issues the same select and the same upsert a client
+ * operation does, but nobody is waiting on it - and on mssql it works the table unindexed,
+ * {@code k} being a {@code varbinary(max)} that cannot be an index key - so bounding an
+ * import as an entry read fails an import that ran to the end before this bound existed.
+ * The catalog lookups of {@code openTree()} keep the operation class whoever runs them: they
+ * read a data dictionary rather than the data, so a wait there is another session's metadata
+ * lock, which is one of the waits this bound exists to end.
+ */
+ final StatementBound bound;
boolean isReadOnly=true;
public ReadableTransactionImpl(Connection con) {
+ this(con, StatementBound.OPERATION);
+ }
+
+ ReadableTransactionImpl(Connection con, StatementBound bound) {
this.con=con;
+ this.bound=bound;
}
@Override
public ByteString read(TreeName treeName, ByteSequence key) {
- try (final PreparedStatement statement=con.prepareStatement("select v from "+readTableName(treeName)+" where h="+hashParam(con)+" and k=?")){
+ // the non-enrolling name: a read must not put a tree this backend does not own - the
+ // shared compressed schema tree of #873 - up for removal
+ final String tableName=readTableName(treeName);
+ try (final PreparedStatement statement=con.prepareStatement("select v from "+tableName+" 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)) {
- return rc.next() ? ByteString.wrap(rc.getBytes("v")) : null;
- }
+ return executeResultSet(statement, bound, rc -> rc.next() ? valueOfRow(rc, tableName) : null);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1353,14 +1923,47 @@
@Override
public Cursor<ByteString, ByteString> openCursor(TreeName treeName) {
- return new CursorImpl(isReadOnly,con,treeName);
+ return new CursorImpl(isReadOnly,con,treeName,bound);
}
+ /**
+ * {@inheritDoc}
+ * <p>
+ * The batches of such a cursor are bulk statements however ordinary they look: nobody is
+ * waiting on the walk, and on mssql it is not even a walk along an index - {@code k} is a
+ * {@code varbinary(max)} there, which cannot be an index key, so every batch is a scan and
+ * a sort of the table. Bounding those as entry reads aborted an export or a rebuild that
+ * ran to the end before this bound existed.
+ */
+ @Override
+ public Cursor<ByteString, ByteString> openBulkCursor(TreeName treeName) {
+ return new CursorImpl(isReadOnly,con,treeName,StatementBound.BULK);
+ }
+
+ /**
+ * {@inheritDoc}
+ * <p>
+ * Bulk whoever asks: {@code select count(*)} is a scan of the whole table on every engine
+ * here, so what it takes follows the size of the backend rather than the work of the caller
+ * that happens to ask. Its callers are administrative either way - {@code dbtest} through
+ * {@code BackendStat}, and the counts {@code verify-index} reports - so the override costs a
+ * client operation nothing. It is not the count behind {@code NOTE_BACKEND_STARTED}: that one
+ * is {@code BackendImpl.getEntryCount()} through {@code RootContainer.getEntryCount()}, which
+ * sums {@code id2childrenCount} and never reaches this method.
+ * <p>
+ * One of the places the class of the transaction is overridden downwards, the others being
+ * {@link #openBulkCursor(TreeName)}, {@link CursorImpl#positionToLastKey()} and the DDL a
+ * write transaction issues - the {@code create table} and the three {@code create index} of
+ * {@code openTree()}, the {@code delete from} of {@code clearTree()} and the {@code drop
+ * table} of {@code deleteTree()} - which is where an operation-class transaction, the one
+ * {@code write()} runs with, can take the shared backstop of its connection off. The count
+ * is deliberately not given here: whoever audits that list has to read it off the class
+ * rather than trust a number that a later hard-coded {@code BULK} would leave stale.
+ */
@Override
public long getRecordCount(TreeName treeName) {
- try (final PreparedStatement statement=con.prepareStatement("select count(*) from "+readTableName(treeName));
- final ResultSet rc=executeResultSet(statement)){
- return rc.next() ? rc.getLong(1) : 0;
+ try (final PreparedStatement statement=con.prepareStatement("select count(*) from "+readTableName(treeName))){
+ return executeResultSet(statement, StatementBound.BULK, rc -> rc.next() ? rc.getLong(1) : 0);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1378,26 +1981,32 @@
// the writeable one inherits it.
boolean isExistsTable(TreeName treeName) {
final String tableName = readTableName(treeName);
+ // the catalog lookup guarding a create table is bounded as the operation it is, not as
+ // the bulk statement it guards, and not as the class of the transaction that happens to
+ // ask: it reads a data dictionary rather than the data, so a wait here is the metadata
+ // lock of another session
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_<hash>" also matches a table named "opendjX<hash>"
- if (tableName.equalsIgnoreCase(rs.getString("TABLE_NAME"))) {
- return true;
+ return bounded(con, StatementBound.OPERATION, () -> {
+ 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_<hash>" also matches a table named "opendjX<hash>"
+ if (tableName.equalsIgnoreCase(rs.getString("TABLE_NAME"))) {
+ return true;
+ }
}
}
- }
+ return false;
+ });
} catch (Exception e) {
throw new StorageRuntimeException(e);
}
- return false;
}
}
/**
@@ -1430,7 +2039,11 @@
boolean partlyCommitted;
public WriteableTransactionTransactionImpl(Connection con) {
- super(con);
+ this(con, StatementBound.OPERATION);
+ }
+
+ WriteableTransactionTransactionImpl(Connection con, StatementBound bound) {
+ super(con, bound);
//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
@@ -1461,8 +2074,12 @@
*/
private void commitStatement(String sql, boolean ddl) throws SQLException {
partlyCommitted|=ddl && commitsBeforeDdl();
+ // Bulk, whatever class the transaction itself carries: every statement issued through here is
+ // one of the ones #877 names as overridden downwards - the create table and the three create
+ // index of openTree(), the delete from of clearTree() and the drop table of deleteTree() -
+ // and nobody is waiting on any of them.
try (final PreparedStatement statement=con.prepareStatement(sql)) {
- execute(statement);
+ execute(statement, StatementBound.BULK);
partlyCommitted=true; // a commit that fails leaves the outcome unknown, which is no more replayable
con.commit();
}
@@ -1544,15 +2161,17 @@
}
boolean isExistsIndex(String tableName, String indexName) throws SQLException {
- // approximate=true: with false the oracle driver runs ANALYZE on every call
- try (final ResultSet rs = con.getMetaData().getIndexInfo(null, null, tableName, false, true)) {
- while (rs.next()) {
- if (indexName.equalsIgnoreCase(rs.getString("INDEX_NAME"))) {
- return true;
+ return bounded(con, StatementBound.OPERATION, () -> {
+ // approximate=true: with false the oracle driver runs ANALYZE on every call
+ try (final ResultSet rs = con.getMetaData().getIndexInfo(null, null, tableName, false, true)) {
+ while (rs.next()) {
+ if (indexName.equalsIgnoreCase(rs.getString("INDEX_NAME"))) {
+ return true;
+ }
}
}
- }
- return false;
+ return false;
+ });
}
public void clearTree(TreeName treeName) {
@@ -1599,28 +2218,28 @@
statement.setString(1, key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2, real2db(key.toByteArray()));
statement.setBytes(3, value.toByteArray());
- return (execute(statement) == 1 && statement.getUpdateCount() > 0);
+ return (execute(statement, bound) == 1 && statement.getUpdateCount() > 0);
}
}else if (driverName.contains("mysql")) { //mysql upsert
try (final PreparedStatement statement = con.prepareStatement("insert into " + getTableName(treeName) + " (h,k,v) values (?,?,?) as new ON DUPLICATE KEY UPDATE v=new.v")) {
statement.setString(1, key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2, real2db(key.toByteArray()));
statement.setBytes(3, value.toByteArray());
- return (execute(statement) == 1 && statement.getUpdateCount() > 0);
+ return (execute(statement, bound) == 1 && statement.getUpdateCount() > 0);
}
}else if (driverName.contains("oracle")) { //ANSI MERGE without ;
try (final PreparedStatement statement = con.prepareStatement("merge into " + getTableName(treeName) + " old using (select ? h,? k,? v from dual) new on (old.h=new.h and old.k=new.k) WHEN MATCHED THEN UPDATE SET old.v=new.v WHEN NOT MATCHED THEN INSERT (h,k,v) VALUES (new.h,new.k,new.v)")) {
statement.setString(1, key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2, real2db(key.toByteArray()));
statement.setBytes(3, value.toByteArray());
- return (execute(statement) == 1 && statement.getUpdateCount() > 0);
+ return (execute(statement, bound) == 1 && statement.getUpdateCount() > 0);
}
}else if (driverName.contains("microsoft")) { //ANSI MERGE with ; WITH (HOLDLOCK) makes the upsert atomic: without it SQL Server MERGE can race two concurrent NOT MATCHED inserts of the same key into a PRIMARY KEY violation. UPDLOCK is required on top of it: with HOLDLOCK alone the search phase takes a shared lock that the WHEN MATCHED update then has to convert to an exclusive one, so two concurrent upserts of the same key deadlock on the conversion; an update lock is taken right away and makes the second transaction wait instead. h is cast back to char so that the join can seek the primary key instead of scanning the whole table under those locks, see hashParam()
try (final PreparedStatement statement = con.prepareStatement("merge into " + getTableName(treeName) + " WITH (HOLDLOCK, UPDLOCK) old using (select cast(? as char(128)) h,? k,? v) new on (old.h=new.h and old.k=new.k) WHEN MATCHED THEN UPDATE SET old.v=new.v WHEN NOT MATCHED THEN INSERT (h,k,v) VALUES (new.h,new.k,new.v);")) {
statement.setString(1, key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(2, real2db(key.toByteArray()));
statement.setBytes(3, value.toByteArray());
- return (execute(statement) == 1 && statement.getUpdateCount() > 0);
+ return (execute(statement, bound) == 1 && statement.getUpdateCount() > 0);
}
}else { //ANSI SQL: try update before insert with not exists
return update(treeName,key,value) || insert(treeName,key,value);
@@ -1634,7 +2253,7 @@
statement.setBytes(3, value.toByteArray());
statement.setString(4, key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(5, real2db(key.toByteArray()));
- return (execute(statement)==1 && statement.getUpdateCount()>0);
+ return (execute(statement, bound)==1 && statement.getUpdateCount()>0);
}
}
@@ -1643,7 +2262,7 @@
statement.setBytes(1,value.toByteArray());
statement.setString(2,key2hash.get(ByteBuffer.wrap(key.toByteArray())));
statement.setBytes(3,real2db(key.toByteArray()));
- return (execute(statement)==1 && statement.getUpdateCount()>0);
+ return (execute(statement, bound)==1 && statement.getUpdateCount()>0);
}
}
@@ -1672,7 +2291,7 @@
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()));
- return (execute(statement)==1 && statement.getUpdateCount()>0);
+ return (execute(statement, bound)==1 && statement.getUpdateCount()>0);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1707,13 +2326,21 @@
ByteString currentValue;
boolean defined;
- public CursorImpl(boolean isReadOnly, Connection con, TreeName treeName) {
+ // The class of the statements this cursor issues, from whoever opened it: a search walks its
+ // index and has a client waiting, while an import, an export or a rebuild walks a whole tree
+ // with nobody waiting - and on mssql it walks it unindexed either way. It is not read off the
+ // shape of the statement, because the opening batch of every cursor is the same
+ // unconditioned "order by k" that positionToLastKey() issues, search or not.
+ final StatementBound batchBound;
+
+ public CursorImpl(boolean isReadOnly, Connection con, TreeName treeName, StatementBound batchBound) {
this.isReadOnly=isReadOnly;
this.con=con;
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.batchBound=batchBound;
this.limitClause=((CachedConnection)con).parent.getClass().getName().contains("mysql")
? " limit ?,?" : " offset ? rows fetch next ? rows only";
}
@@ -1724,7 +2351,14 @@
return size;
}
- boolean fetchBatch(String condition, byte[] dbKey, long offset, boolean descending, int limit) {
+ /**
+ * Reads one batch of the cursor. The class of the bound is the caller's: a batch taken
+ * along the index of the tree for a client is an operation, while a batch that has to look
+ * at the whole table to answer - the one behind {@link #positionToLastKey()} - and every
+ * batch of a cursor an import or a rebuild walks ({@link #batchBound}) is bulk work, and
+ * the two cannot share a value.
+ */
+ boolean fetchBatch(String condition, byte[] dbKey, long offset, boolean descending, int limit, StatementBound bound) {
fetchCount++;
buffer.clear();
try (final PreparedStatement statement=con.prepareStatement("select k,v from "+tableName
@@ -1736,15 +2370,15 @@
}
statement.setLong(i++,offset);
statement.setLong(i,limit);
- try(final ResultSet rc=executeResultSet(statement)) {
+ return executeResultSet(statement, bound, rc -> {
while (rc.next()) {
- buffer.add(new byte[][]{rc.getBytes(1),rc.getBytes(2)});
+ buffer.add(new byte[][]{rc.getBytes(1),valueOfRow(rc.getBytes(2),tableName)});
}
- }
+ return !buffer.isEmpty();
+ });
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
- return !buffer.isEmpty();
}
void advanceFromBuffer() {
@@ -1757,7 +2391,7 @@
@Override
public boolean next() {
- if (buffer.isEmpty() && !fetchBatch(currentKeyDb==null?null:">",currentKeyDb,0,false,adaptiveBatchSize())) {
+ if (buffer.isEmpty() && !fetchBatch(currentKeyDb==null?null:">",currentKeyDb,0,false,adaptiveBatchSize(),batchBound)) {
defined=false;
return false;
}
@@ -1802,7 +2436,7 @@
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);
+ execute(statement, batchBound);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
@@ -1832,7 +2466,7 @@
if (!buffer.isEmpty()) { // jumped outside the buffered range: random access, back to small batches
nextBatchSize=initialBatchSize;
}
- if (fetchBatch(">=",target,0,false,adaptiveBatchSize())) {
+ if (fetchBatch(">=",target,0,false,adaptiveBatchSize(),batchBound)) {
advanceFromBuffer();
return true;
}
@@ -1843,30 +2477,43 @@
@Override
public boolean positionToKey(ByteSequence key) {
final byte[] real=key.toByteArray();
+ // The row is wrapped inside the handler rather than after it, so that null keeps meaning
+ // "no such key" and only that: a row whose v is null - which the schema allows, however
+ // this backend writes it - has to fail here as it fails in read(), rather than report a
+ // key that exists as absent. Both go through valueOfRow(), which is where that failure
+ // is named.
+ final ByteString value;
try (final PreparedStatement statement=con.prepareStatement("select v from "+tableName+" where h="+hashParam(con)+" and k=?")){
statement.setString(1,key2hash.get(ByteBuffer.wrap(real)));
statement.setBytes(2,real2db(real));
- try(final ResultSet rc=executeResultSet(statement)) {
- if (rc.next()) {
- buffer.clear();
- nextBatchSize=initialBatchSize;
- currentKeyDb=real2db(real);
- currentKey=ByteString.wrap(real);
- currentValue=ByteString.wrap(rc.getBytes("v"));
- defined=true;
- return true;
- }
- }
+ value=executeResultSet(statement, batchBound, rc -> rc.next() ? valueOfRow(rc, tableName) : null);
}catch (SQLException e) {
throw new StorageRuntimeException(e);
}
+ if (value!=null) {
+ buffer.clear();
+ nextBatchSize=initialBatchSize;
+ currentKeyDb=real2db(real);
+ currentKey=ByteString.wrap(real);
+ currentValue=value;
+ defined=true;
+ return true;
+ }
defined=false;
return false;
}
+ /**
+ * Bulk, not operation: with no condition to seek on, this is {@code order by k desc} over
+ * the whole table - and on mssql, where {@code k} is a {@code varbinary(max)} that cannot
+ * be an index key, a scan and a sort of it. It is also not on a search path: every open of
+ * a backend runs it once per base DN, through {@code EntryContainer.getHighestEntryID()},
+ * outside the try/catch of {@code BackendImpl.openBackend()} - a bound of two minutes here
+ * would turn a large backend that opens slowly into one that does not open at all.
+ */
@Override
public boolean positionToLastKey() {
- if (fetchBatch(null,null,0,true,1)) {
+ if (fetchBatch(null,null,0,true,1,StatementBound.BULK)) {
advanceFromBuffer();
return true;
}
@@ -1874,12 +2521,20 @@
return false;
}
+ /**
+ * The class of the cursor, unlike {@link #positionToLastKey()}, which is bulk however it
+ * was opened: an offset comes from the VLV request of a client, so this runs on a search
+ * path and has to give the worker thread back - an import has no VLV position to seek to.
+ * That a deep offset is served by walking to it - the engines have no other way to answer
+ * an {@code offset ?} - is what makes the bound reachable here, and reaching it answers the
+ * request with an error rather than parking a thread of the server on it.
+ */
@Override
public boolean positionToIndex(int index) {
if (!buffer.isEmpty()) { // absolute jump: random access, back to small batches
nextBatchSize=initialBatchSize;
}
- if (index>=0 && fetchBatch(null,null,index,false,adaptiveBatchSize())) {
+ if (index>=0 && fetchBatch(null,null,index,false,adaptiveBatchSize(),batchBound)) {
advanceFromBuffer();
return true;
}
@@ -1893,7 +2548,7 @@
return tree2table.asMap().keySet();
}
- private final class ImporterImpl implements Importer {
+ final class ImporterImpl implements Importer {
final Connection con;
final ReadableTransactionImpl txr;
final WriteableTransactionTransactionImpl txw;
@@ -1913,22 +2568,32 @@
final Boolean isOpen;
- public ImporterImpl() {
- isOpen=getStorageStatus().isWorking();
- if (!isOpen) {
- try {
- open(AccessMode.READ_WRITE);
- }catch (Exception e) {
- throw new StorageRuntimeException(e);
- }
+ /**
+ * Both transactions of an import take the bulk class, and with them every statement it
+ * issues: phase one writes the trees through {@code put()}, phase two reads them back
+ * through {@code read()} and walks them through {@code openCursor()}, and none of that has
+ * a client waiting on it. Bounding those as entry reads is not merely strict, it fails work
+ * that ran to the end before this bound existed: {@code h} is the primary key on every
+ * dialect and the default lock wait is forever on mssql, postgres and oracle, so an upsert
+ * of an online import blocked by an LDAP write on the same table sat until the bound of an
+ * entry read and then failed the import.
+ */
+ ImporterImpl(Connection con, boolean isOpen) {
+ // An import writes by definition, so a storage that is not writeable refuses one where the
+ // importer is built - which is where it was refused until the write transaction of a read-only
+ // storage became one that is granted and checks per operation (#874). Left to that check, an
+ // import of such a storage would take a connection out of the pool, begin its transaction and
+ // fail at the first tree it clears rather than at its start.
+ // What arrives here read-only is a storage that was already open: import-ldif and
+ // rebuild-index both close it first, and startImport() opens a closed one READ_WRITE - an
+ // import of any storage of this server reopens it that way - so those two arrive writeable.
+ if (!accessMode.isWriteable()) {
+ throw new ReadOnlyStorageException();
}
- try {
- con = getValidatedConnection();
- }catch (Exception e){
- throw new StorageRuntimeException(e);
- }
- txr =new ReadableTransactionImpl(con);
- txw =new WriteableTransactionTransactionImpl(con);
+ this.con=con;
+ this.isOpen=isOpen;
+ txr=new ReadableTransactionImpl(con, StatementBound.BULK);
+ txw=new WriteableTransactionTransactionImpl(con, StatementBound.BULK);
}
@Override
@@ -1936,6 +2601,11 @@
aborted = true;
}
+ // The connection goes back whatever the commit does, and the storage this importer opened
+ // is closed whatever the connection does: an importer is closed on the way out of a failed
+ // import as readily as a finished one - a clearTree() that reaches the bulk bound is one
+ // way there - and a commit that throws on the way would otherwise leave the connection
+ // out of the pool for good, holding the transaction and the locks of that import.
@Override
public void close() {
try {
@@ -1979,6 +2649,11 @@
return txr.read(treeName, key);
}
+ // Bulk like every other statement of an import, by the class of the transaction it comes
+ // from: this walks a whole tree with no client waiting on it - phase one of a rebuild-index
+ // reads every record of id2entry through this cursor (OnDiskMergeImporter.ID2EntrySource) -
+ // and on mssql it walks it unindexed, so a batch of it is a scan and a sort of the table
+ // rather than a step along an index.
@Override
public SequentialCursor<ByteString, ByteString> openCursor(TreeName treeName) {
return txr.openCursor(treeName);
@@ -1988,7 +2663,52 @@
//import
@Override
public Importer startImport() throws ConfigException, StorageRuntimeException {
- return new ImporterImpl();
+ final boolean wasOpen=getStorageStatus().isWorking();
+ if (!wasOpen) {
+ try {
+ open(AccessMode.READ_WRITE);
+ }catch (Exception e) {
+ throw new StorageRuntimeException(e);
+ }
+ }
+ final Connection con;
+ try {
+ con=getValidatedConnection();
+ }catch (Exception e){
+ // and the storage this method opened goes back with it: ImporterImpl.close() is what closes
+ // it again when an import opened it, and no importer is going to be built to reach that
+ if (!wasOpen) {
+ close();
+ }
+ throw new StorageRuntimeException(e);
+ }
+ // outside the catch: the importer of a read-only storage throws ReadOnlyStorageException,
+ // which a caller tells apart from any other failure of an import
+ boolean built=false;
+ try {
+ final Importer importer=new ImporterImpl(con, wasOpen);
+ built=true;
+ return importer;
+ }finally {
+ // and the connection borrowed above goes back on every path that does not build an
+ // importer to hold it: it is the one an import keeps for its whole duration, so leaving it
+ // here takes it out of the pool for good, with the transaction it had already begun. A
+ // finally rather than a catch, so that it covers what a catch has to name - an Error
+ // leaves the pool one connection short exactly as ReadOnlyStorageException did.
+ if (!built) {
+ try {
+ con.close();
+ }catch (SQLException ignored) {
+ // the importer was never built; the failure to report is the one on its way out
+ }
+ // and the storage this method opened goes back with the connection, for the reason the
+ // borrow above gives: ImporterImpl.close() is what closes it again when an import
+ // opened it, and there is no importer here to reach that
+ if (!wasOpen) {
+ close();
+ }
+ }
+ }
}
//backup
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendStat.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendStat.java
index 694edba..dbee294 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendStat.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/BackendStat.java
@@ -1118,7 +1118,8 @@
long undefined = 0;
long count = 0;
BackendTreeKeyValue keyDecoder = new BackendTreeKeyValue(index);
- try (Cursor<ByteString, EntryIDSet> cursor = index.openCursor(txn))
+ // dbtest walks the index whole, on the command line of an operator: bulk work either way
+ try (Cursor<ByteString, EntryIDSet> cursor = index.openBulkCursor(txn))
{
while (cursor.next())
{
@@ -1286,7 +1287,8 @@
long count = 0;
long totalKeySize = 0;
long totalDataSize = 0;
- try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(target.getTreeName()))
+ // dbtest walks the tree whole, on the command line of an operator: bulk work either way
+ try (final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(target.getTreeName()))
{
ByteString key;
ByteString maxKey = null;
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java
index 99faae0..88edf18 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/DefaultIndex.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.backends.pluggable;
@@ -131,7 +132,19 @@
public final Cursor<ByteString, EntryIDSet> openCursor(ReadableTransaction txn)
{
checkNotNull(txn, "txn must not be null");
- return CursorTransformer.transformValues(txn.openCursor(getName()),
+ return decoding(txn.openCursor(getName()));
+ }
+
+ @Override
+ public final Cursor<ByteString, EntryIDSet> openBulkCursor(ReadableTransaction txn)
+ {
+ checkNotNull(txn, "txn must not be null");
+ return decoding(txn.openBulkCursor(getName()));
+ }
+
+ private Cursor<ByteString, EntryIDSet> decoding(Cursor<ByteString, ByteString> cursor)
+ {
+ return CursorTransformer.transformValues(cursor,
new ValueTransformer<ByteString, ByteString, EntryIDSet, NeverThrowsException>()
{
@Override
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 e9188ab..a7ae259 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
@@ -2380,7 +2380,29 @@
long getNumberOfEntriesInBaseDN0(ReadableTransaction txn)
{
- return id2childrenCount.getTotalCount(txn);
+ return getNumberOfEntriesInBaseDN0(txn, false);
+ }
+
+ /**
+ * The same count, told which kind of work it is part of: {@code verify-index} reads it before
+ * walking the whole backend, with nobody waiting on the walk or on the count that sizes it, while
+ * {@code cn=monitor} and the searches of {@code GroupManager} and {@code SubentryManager} read it
+ * for a client.
+ * <p>
+ * The read behind {@code NOTE_BACKEND_STARTED} is deliberately left with the client callers,
+ * though nobody waits on it either: it goes through {@code BackendImpl.getEntryCount()}, which is
+ * the same method those three call, and what a bound costs it there is a log line reporting -1
+ * entries - that method answers -1 for any failure rather than failing the open.
+ *
+ * @param txn storage transaction
+ * @param partOfAWholeTreeWalk whether this read belongs to a walk of a whole tree rather than to
+ * a client operation
+ * @return The number of entries stored in this entry container including the baseDN.
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ long getNumberOfEntriesInBaseDN0(ReadableTransaction txn, boolean partOfAWholeTreeWalk)
+ {
+ return id2childrenCount.getTotalCount(txn, partOfAWholeTreeWalk);
}
/**
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ExportJob.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ExportJob.java
index aedf099..75e2fd2 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ExportJob.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ExportJob.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2008 Sun Microsystems, Inc.
* Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.backends.pluggable;
@@ -168,11 +169,17 @@
* @throws LDIFException If an error occurs while trying to determine
* whether to write an entry.
*/
- private void exportContainer(ReadableTransaction txn, EntryContainer entryContainer)
+ // Visible to the test that pins the class of the cursor opened here: a walk of the whole of
+ // id2entry with nobody waiting on it, which a storage engine that bounds a statement must not
+ // bound as it bounds an operation (#877).
+ void exportContainer(ReadableTransaction txn, EntryContainer entryContainer)
throws StorageRuntimeException, IOException, LDIFException
{
ID2Entry id2entry = entryContainer.getID2Entry();
- try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(id2entry.getName()))
+ // The whole of id2entry with nobody waiting on the walk: an export-ldif, or the generation ID
+ // a replicated domain computes for itself the first time it starts (LDAPReplicationDomain
+ // .computeGenerationId), which is why this must not be bounded as the work of an operation.
+ try (final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(id2entry.getName()))
{
while (cursor.next())
{
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java
index 812300f..7eb800c 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ID2ChildrenCount.java
@@ -57,9 +57,16 @@
this.counter = new ShardedCounter(name);
}
- SequentialCursor<EntryID, Void> openCursor(ReadableTransaction txn)
+ /**
+ * Walks the children counts whole, which {@code verify-index} does and no client operation does:
+ * there is no overload of this method that would take the bound of an operation by accident.
+ * Reading the count of a single entry is another matter - see {@link #getCount}.
+ *
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ SequentialCursor<EntryID, Void> openBulkCursor(ReadableTransaction txn)
{
- return transformKeysAndValues(counter.openCursor(txn),
+ return transformKeysAndValues(counter.openBulkCursor(txn),
TO_ENTRY_ID, CursorTransformer.<ByteString, Void> keepValuesUnchanged());
}
@@ -141,7 +148,23 @@
*/
long getCount(ReadableTransaction txn, EntryID entryID)
{
- return counter.getCount(txn, toKey(entryID));
+ return getCount(txn, entryID, false);
+ }
+
+ /**
+ * Get the number of children for the given entry, as part of a walk of a whole tree rather than
+ * of a client operation. {@code verify-index} reads one of these per DN while it walks dn2id
+ * whole, and no client is waiting on any of them.
+ *
+ * @param txn storage transaction
+ * @param entryID The entryID identifying to the counter
+ * @param partOfAWholeTreeWalk whether this read belongs to a walk of a whole tree
+ * @return Value of the counter. 0 if no counter is associated yet.
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ long getCount(ReadableTransaction txn, EntryID entryID, boolean partOfAWholeTreeWalk)
+ {
+ return counter.getCount(txn, toKey(entryID), partOfAWholeTreeWalk);
}
/**
@@ -151,7 +174,26 @@
*/
long getTotalCount(ReadableTransaction txn)
{
- return getCount(txn, TOTAL_COUNT_ENTRY_ID);
+ return getTotalCount(txn, false);
+ }
+
+ /**
+ * The same total, told which kind of work it is part of. It is a read of this tree like any
+ * other - a cursor positioned on one key, which on a storage engine that walks a table rather
+ * than an index is a scan of it - so what it may take follows who is waiting on it:
+ * {@code verify-index} reads it once to size the progress report of a walk of the whole backend,
+ * with nobody waiting, while {@code cn=monitor} and the searches of {@code GroupManager} and
+ * {@code SubentryManager} read the same total for a client.
+ *
+ * @param txn storage transaction
+ * @param partOfAWholeTreeWalk whether this read belongs to a walk of a whole tree rather than to
+ * a client operation
+ * @return Sum of all the counter contained in this tree
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ long getTotalCount(ReadableTransaction txn, boolean partOfAWholeTreeWalk)
+ {
+ return getCount(txn, TOTAL_COUNT_ENTRY_ID, partOfAWholeTreeWalk);
}
/**
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 72bbded..af0f312 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
@@ -382,7 +382,11 @@
{
// Make sure the tree is there and readable, even if the storage is READ_ONLY.
// Would be nice if there were a better way...
- try (final Cursor<ByteString, ByteString> cursor = txn.openCursor(getName()))
+ // Bulk: the first batch of a cursor carries no seek predicate, so this is a walk of the whole
+ // tree as far as the storage is concerned, and it runs on every open of the backend. A bound
+ // meant for an entry read would keep a large backend from opening at all on an engine where
+ // such a batch is not a step along an index.
+ try (final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(getName()))
{
cursor.next();
}
@@ -498,6 +502,12 @@
/**
* Check that a record entry exists in the entry tree.
+ * <p>
+ * Bulk, like the walk it belongs to: {@code VerifyJob.iterateID2ChildrenCount()} is its one
+ * caller and asks this once per record of the children count tree, inside a cursor over the
+ * whole of it. Read as a client operation those would put the bound of an entry read over a
+ * job nobody is waiting on, once per record - the same hazard the walk around them was given
+ * {@link ReadableTransaction#openBulkCursor(TreeName)} for (#877).
*
* @param txn a non null transaction
* @param entryID The entry ID which forms the key.
@@ -508,7 +518,7 @@
{
checkNotNull(txn, "txn must not be null");
checkNotNull(entryID, "entryID must not be null");
- try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(getName())) {
+ try(final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(getName())) {
return cursor.positionToKey(entryID.toByteString());
}
}
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Index.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Index.java
index bbaf6b5..078348d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Index.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/Index.java
@@ -13,6 +13,7 @@
*
* Copyright 2006-2010 Sun Microsystems, Inc.
* Portions Copyright 2012-2016 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.backends.pluggable;
@@ -37,6 +38,23 @@
Cursor<ByteString, EntryIDSet> openCursor(ReadableTransaction txn);
+ /**
+ * Opens a cursor over the whole index for a task no client operation is waiting on, such as
+ * {@code verify-index} or {@code dbtest}.
+ * <p>
+ * Abstract rather than a {@code default} answering as {@link #openCursor(ReadableTransaction)}
+ * does, which is the compatibility the SPI needs for engines outside this repository: this
+ * interface is package-private with one implementor, and a second one inheriting that default
+ * would silently walk a whole index under the bound of a client operation. A compile error is
+ * the better answer here.
+ *
+ * @param txn
+ * the transaction to read the index with
+ * @return a cursor over every key of this index
+ * @see ReadableTransaction#openBulkCursor(org.opends.server.backends.pluggable.spi.TreeName)
+ */
+ Cursor<ByteString, EntryIDSet> openBulkCursor(ReadableTransaction txn);
+
boolean setIndexEntryLimit(int indexEntryLimit);
boolean setConfidential(boolean indexConfidential);
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 0647565..08f236c 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
@@ -349,7 +349,7 @@
return 0;
}
long copied = 0;
- try (Cursor<ByteString, ByteString> cursor = txn.openCursor(from))
+ try (Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(from))
{
while (cursor.next())
{
@@ -375,9 +375,10 @@
// 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.
+ // Both trees are read whole while the backend opens, with no client operation waiting on it.
if (txn.treeExists(ocTree))
{
- try (Cursor<ByteString, ByteString> ocCursor = txn.openCursor(ocTree))
+ try (Cursor<ByteString, ByteString> ocCursor = txn.openBulkCursor(ocTree))
{
while (ocCursor.next())
{
@@ -403,7 +404,7 @@
// Cursor through the attribute description database and load the attribute set definitions.
if (txn.treeExists(adTree))
{
- try (Cursor<ByteString, ByteString> adCursor = txn.openCursor(adTree))
+ try (Cursor<ByteString, ByteString> adCursor = txn.openBulkCursor(adTree))
{
while (adCursor.next())
{
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ShardedCounter.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ShardedCounter.java
index efde5df..9e5769d 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ShardedCounter.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/ShardedCounter.java
@@ -12,6 +12,7 @@
* information: "Portions Copyright [year] [name of copyright owner]".
*
* Copyright 2015 ForgeRock AS.
+ * Portions Copyright 2026 3A Systems, LLC.
*/
package org.opends.server.backends.pluggable;
@@ -72,16 +73,30 @@
super(name);
}
- SequentialCursor<ByteString, Void> openCursor(ReadableTransaction txn)
+ /**
+ * Walks this counter whole, which {@code verify-index} does and no client operation does: there
+ * is no overload of this method that would take the bound of an operation by accident. Reading a
+ * single counter is another matter - {@link #getCount} opens a cursor of its own, and one of a
+ * client operation unless the caller says otherwise.
+ *
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ SequentialCursor<ByteString, Void> openBulkCursor(ReadableTransaction txn)
+ {
+ return uniqueKeys(txn.openBulkCursor(getName()));
+ }
+
+ private SequentialCursor<ByteString, Void> uniqueKeys(Cursor<ByteString, ByteString> cursor)
{
return new UniqueKeysCursor<>(transformKeysAndValues(
- txn.openCursor(getName()), TO_KEY,
+ cursor, TO_KEY,
CursorTransformer.<ByteString, ByteString, Void> constant(null)));
}
- private Cursor<ByteString, Long> openCursor0(ReadableTransaction txn)
+ private Cursor<ByteString, Long> openCursor0(ReadableTransaction txn, boolean partOfAWholeTreeWalk)
{
- return transformKeysAndValues(txn.openCursor(getName()), TO_KEY, TO_LONG);
+ return transformKeysAndValues(
+ partOfAWholeTreeWalk ? txn.openBulkCursor(getName()) : txn.openCursor(getName()), TO_KEY, TO_LONG);
}
void addCount(final WriteableTransaction txn, ByteSequence key, final long delta)
@@ -107,8 +122,35 @@
long getCount(final ReadableTransaction txn, ByteSequence key)
{
+ return getCount(txn, key, false);
+ }
+
+ /**
+ * The same read, told which kind of work it is part of. A client operation reads a counter of its
+ * own and takes the bound of one - {@code numSubordinates} of a search
+ * ({@code EntryContainer.getNumberOfChildren}), the entry count of a VLV index a search is paging
+ * through ({@code VLVIndex.getEntryCount}) - while {@code verify-index} reads one per DN of the
+ * tree it is walking, with nobody waiting on it: bounding those as client operations is what #877
+ * exists to stop, and on the JDBC backend it aborted a verify of a backend large enough.
+ * <p>
+ * The third caller is {@code ID2ChildrenCount.getTotalCount}, which is read both ways and is told
+ * which it is by its own caller: a verify sizes its progress report with it, {@code cn=monitor}
+ * and the searches of {@code GroupManager} and {@code SubentryManager} read it for a client. A
+ * delete and a modify DN reach neither form - they go through {@link #removeCount}, which is a
+ * client operation by construction.
+ *
+ * @param txn storage transaction
+ * @param key the counter to read
+ * @param partOfAWholeTreeWalk whether this read belongs to a walk of a whole tree rather than to
+ * a client operation
+ * @return Value of the counter. 0 if no counter is associated yet.
+ * @see ReadableTransaction#openBulkCursor(TreeName)
+ */
+ long getCount(final ReadableTransaction txn, ByteSequence key, boolean partOfAWholeTreeWalk)
+ {
long counterValue = 0;
- try (final SequentialCursor<ByteString, Long> cursor = new ShardCursor(openCursor0(txn), key))
+ try (final SequentialCursor<ByteString, Long> cursor =
+ new ShardCursor(openCursor0(txn, partOfAWholeTreeWalk), key))
{
while (cursor.next())
{
@@ -121,7 +163,8 @@
long removeCount(final WriteableTransaction txn, ByteSequence key)
{
long counterValue = 0;
- try (final SequentialCursor<ByteString, Long> cursor = new ShardCursor(openCursor0(txn), key))
+ // a removal is always a client operation: an entry is being deleted or moved
+ try (final SequentialCursor<ByteString, Long> cursor = new ShardCursor(openCursor0(txn, false), key))
{
// Iterate over and remove all the thread local shards
while (cursor.next())
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 42ff1da..b4557d8 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
@@ -306,6 +306,15 @@
}
@Override
+ public Cursor<ByteString, ByteString> openBulkCursor(final TreeName name)
+ {
+ traceEnter("openBulkCursor", "name", name);
+ final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(name);
+ traceLeave("openBulkCursor", "name", name);
+ return new TracedCursor(cursor);
+ }
+
+ @Override
public ByteString read(final TreeName name, final ByteSequence key)
{
traceEnter("read", "name", name, "key", hex(key));
@@ -393,6 +402,15 @@
}
@Override
+ public Cursor<ByteString, ByteString> openBulkCursor(final TreeName name)
+ {
+ traceEnter("openBulkCursor", "name", name);
+ final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(name);
+ traceLeave("openBulkCursor", "name", name);
+ return new TracedCursor(cursor);
+ }
+
+ @Override
public void openTree(final TreeName name, boolean createOnDemand)
{
traceEnter("openTree", "name", name, "createOnDemand", createOnDemand);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
index 6784d24..06d4b83 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/VerifyJob.java
@@ -92,12 +92,15 @@
/** Indicates whether the children count tree is to be verified. */
private boolean verifyID2ChildrenCount;
+ // The trees below, and the iterate* methods that walk them, are visible to the test pinning the
+ // class of the cursors they open: every one of these walks a tree whole with nobody waiting on
+ // it, which a storage engine that bounds a statement must not bound as an operation (#877).
/** The entry tree. */
- private ID2Entry id2entry;
+ ID2Entry id2entry;
/** The DN tree. */
- private DN2ID dn2id;
+ DN2ID dn2id;
/** The children tree. */
- private ID2ChildrenCount id2childrenCount;
+ ID2ChildrenCount id2childrenCount;
/** A list of the attribute indexes to be verified. */
private final ArrayList<AttributeIndex> attrIndexList = new ArrayList<>();
@@ -343,9 +346,10 @@
*
* @throws StorageRuntimeException If an error occurs in the storage.
*/
- private void iterateID2Entry(ReadableTransaction txn) throws StorageRuntimeException
+ void iterateID2Entry(ReadableTransaction txn) throws StorageRuntimeException
{
- try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(id2entry.getName()))
+ // Every tree this job walks, it walks whole, and no client operation is waiting on it.
+ try(final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(id2entry.getName()))
{
long storedEntryCount = id2entry.getRecordCount(txn);
while (cursor.next())
@@ -437,12 +441,12 @@
*
* @throws StorageRuntimeException If an error occurs in the storage.
*/
- private void iterateDN2ID(ReadableTransaction txn) throws StorageRuntimeException
+ void iterateDN2ID(ReadableTransaction txn) throws StorageRuntimeException
{
final Deque<ChildrenCount> childrenCounters = new LinkedList<>();
ChildrenCount currentNode = null;
- try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(dn2id.getName()))
+ try(final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(dn2id.getName()))
{
while (cursor.next())
{
@@ -515,7 +519,10 @@
private void verifyID2ChildrenCount(ReadableTransaction txn, ChildrenCount parent) {
final long expected = parent.numberOfChildren;
- final long currentValue = id2childrenCount.getCount(txn, parent.entryID);
+ // Part of the walk of dn2id above, and bounded as one: this runs once per DN of the tree, so
+ // reading it as a client operation would put the bound of an entry read over a read of a
+ // backend nobody is waiting on - which on the JDBC backend aborts a verify of a large one.
+ final long currentValue = id2childrenCount.getCount(txn, parent.entryID, true);
if (expected != currentValue)
{
errorCount++;
@@ -525,7 +532,7 @@
private void iterateID2ChildrenCount(ReadableTransaction txn) throws StorageRuntimeException
{
- try (final SequentialCursor<EntryID, Void> cursor = id2childrenCount.openCursor(txn))
+ try (final SequentialCursor<EntryID, Void> cursor = id2childrenCount.openBulkCursor(txn))
{
while (cursor.next())
{
@@ -599,7 +606,7 @@
* @throws StorageRuntimeException If an error occurs in the storage.
* @throws DirectoryException If an error occurs reading values in the index.
*/
- private void iterateVLVIndex(ReadableTransaction txn, VLVIndex vlvIndex, boolean verifyID)
+ void iterateVLVIndex(ReadableTransaction txn, VLVIndex vlvIndex, boolean verifyID)
throws StorageRuntimeException, DirectoryException
{
if(vlvIndex == null || !verifyID)
@@ -607,7 +614,7 @@
return;
}
- try(final Cursor<ByteString, ByteString> cursor = txn.openCursor(vlvIndex.getName()))
+ try(final Cursor<ByteString, ByteString> cursor = txn.openBulkCursor(vlvIndex.getName()))
{
while (cursor.next())
{
@@ -655,7 +662,7 @@
return;
}
- try(final Cursor<ByteString,EntryIDSet> cursor = index.openCursor(txn))
+ try(final Cursor<ByteString,EntryIDSet> cursor = index.openBulkCursor(txn))
{
while (cursor.next())
{
@@ -985,8 +992,13 @@
}
}
- /** This class reports progress of the verify job at fixed intervals. */
- private final class ProgressTask extends TimerTask
+ /**
+ * This class reports progress of the verify job at fixed intervals.
+ * <p>
+ * Visible, with its constructor, to the test pinning the class of the reads it makes to size
+ * that report: they belong to the walk they measure rather than to a client operation (#877).
+ */
+ final class ProgressTask extends TimerTask
{
/** The total number of records to process. */
private long totalCount;
@@ -1001,7 +1013,7 @@
* through indexes or the entries.
* @throws StorageRuntimeException An error occurred while accessing the storage.
*/
- private ProgressTask(boolean indexIterator, ReadableTransaction txn) throws StorageRuntimeException
+ ProgressTask(boolean indexIterator, ReadableTransaction txn) throws StorageRuntimeException
{
previousTime = System.currentTimeMillis();
@@ -1031,7 +1043,12 @@
}
else
{
- totalCount = rootContainer.getEntryContainer(verifyConfig.getBaseDN()).getNumberOfEntriesInBaseDN0(txn);
+ // Part of this walk, like the counts of the branch above: it sizes a verify of the whole
+ // backend, and the branch above is only reached by "verify-index --clean" - a plain
+ // verify-index, with or without an index named, comes here. Read as a client operation it
+ // would take the bound of one on a storage engine that bounds a statement, which on a
+ // backend large enough aborts the verify before its first record (#877).
+ totalCount = rootContainer.getEntryContainer(verifyConfig.getBaseDN()).getNumberOfEntriesInBaseDN0(txn, true);
}
}
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 c846687..208753d 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
@@ -45,6 +45,26 @@
Cursor<ByteString, ByteString> openCursor(TreeName treeName);
/**
+ * Opens a cursor on the tree whose name is provided, for a walk of that whole tree with no
+ * client operation waiting on it: an export, a verify, a rebuild, or the load of a tree while
+ * the backend opens.
+ * <p>
+ * A storage engine that bounds how long a statement may take must not bound such a walk as it
+ * bounds the work of a client operation: what this legitimately takes follows the size of the
+ * tree, and cutting it short fails an administrative task that would otherwise have run to the
+ * end. An engine with no such bound - every one but the JDBC backend - answers this exactly as
+ * {@link #openCursor(TreeName)} does.
+ *
+ * @param treeName
+ * the tree name
+ * @return a new cursor
+ */
+ default Cursor<ByteString, ByteString> openBulkCursor(TreeName treeName)
+ {
+ return openCursor(treeName);
+ }
+
+ /**
* Returns the number of key/value pairs in the provided tree.
*
* @param treeName
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java
new file mode 100644
index 0000000..7c0228b
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java
@@ -0,0 +1,1170 @@
+/*
+ * 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.jdbc;
+
+import org.forgerock.i18n.LocalizableMessage;
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.server.config.server.JDBCBackendCfg;
+import org.mockito.InOrder;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.backends.jdbc.JDBCStorage.StatementBound;
+import org.opends.server.backends.pluggable.spi.AccessMode;
+import org.opends.server.backends.pluggable.spi.ReadOnlyStorageException;
+import org.opends.server.backends.pluggable.spi.StorageRuntimeException;
+import org.opends.server.backends.pluggable.spi.StorageStatus;
+import org.opends.server.backends.pluggable.spi.TreeName;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.SQLFeatureNotSupportedException;
+import java.sql.SQLTimeoutException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Executor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static java.util.Collections.singletonList;
+import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyInt;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.times;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertSame;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+/**
+ * Which bound a statement of the JDBC backend is given, and what reaching it looks like to the
+ * caller (#877). Needs no database: the statement is a mock, so the policy is pinned wherever the
+ * build runs, while the container suites cover a statement really blocked on a lock.
+ */
+@SuppressWarnings("javadoc")
+// Every wait of this suite is bounded where it is taken - awaitOrFail() and Background.joinOrFail()
+// - rather than by a timeOut on this annotation: a method carrying a @Test of its own replaces the
+// one of the class outright, only the groups of the two being merged, so a timeOut declared here
+// would bound nothing. A statement of another thread that never arrives fails this suite there.
+@Test(groups = { "precommit", "jdbc" }, sequential = true)
+public class JDBCStatementBoundTestCase extends DirectoryServerTestCase {
+
+ private JDBCStorage storage;
+
+ /**
+ * A storage of its own for every test. Not merely tidy: a storage carries latches that are
+ * meant to be one-shot for its whole life - the driver having no network timeout, and the
+ * warnings for that and for a refused query timeout - so a test that trips one on a shared
+ * instance would leave {@code applyBackstop()} returning at its first guard for every test
+ * after it, turning their {@code verify(con, never())} assertions green on a run that reached
+ * nothing. Constructing one costs a mock configuration and no connection at all.
+ */
+ @BeforeMethod
+ public void createStorage() {
+ storage = new JDBCStorage(mockCfg(JDBCBackendCfg.class), null);
+ }
+
+ @AfterMethod
+ public void clearProperties() {
+ for (final StatementBound bound : StatementBound.values()) {
+ System.clearProperty(bound.property);
+ }
+ System.clearProperty(JDBCStorage.STATISTICS_TIMEOUT_PROPERTY);
+ storage.accessMode = AccessMode.READ_ONLY; // an import test opens it for writing
+ }
+
+ /** How long a test waits for a statement running on another thread before it fails. */
+ private static final long WAIT_MILLIS = 30000;
+
+ /**
+ * A storage whose clock the test moves by hand. Whether a failure is the bound arriving turns
+ * on a few milliseconds either side of it, and a sleep cannot pin that: a loaded box lengthens
+ * one, so a test that oversleeps passes whether the slack under the bound exists or not - and
+ * a bound of a second costs the suite a second of waiting to say so.
+ */
+ private static final class SteppedClockStorage extends JDBCStorage {
+ /** Milliseconds since the statement under test started, as the classification will see it. */
+ volatile long millis;
+
+ SteppedClockStorage() {
+ super(mockCfg(JDBCBackendCfg.class), null);
+ }
+
+ @Override
+ long nanoTime() {
+ return millis * 1000000L;
+ }
+ }
+
+ private static void awaitOrFail(CountDownLatch latch, String what) throws InterruptedException {
+ assertTrue(latch.await(WAIT_MILLIS, TimeUnit.MILLISECONDS), what + " within " + WAIT_MILLIS + " ms");
+ }
+
+ private static void sleep(long millis) {
+ try {
+ Thread.sleep(millis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+
+ /** A statement that reports it is running and then waits for the test to let it finish. */
+ private PreparedStatement lingering(Connection con, CountDownLatch running, CountDownLatch mayFinish)
+ throws SQLException {
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ running.countDown();
+ awaitOrFail(mayFinish, "the statement was never let go");
+ return 1;
+ }
+ });
+ return statement;
+ }
+
+ private interface Execution {
+ void run() throws Exception;
+ }
+
+ /**
+ * A statement running on a thread of its own, with whatever it threw kept for the assertion:
+ * {@code Thread.join()} does not rethrow, so a failure in the background would otherwise leave
+ * the verifications of a test passing on a run that never reached the state they check.
+ */
+ private static final class Background {
+ final Thread thread;
+ final AtomicReference<Throwable> failure = new AtomicReference<>();
+
+ Background(String name, final Execution execution) {
+ thread = new Thread(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ execution.run();
+ } catch (Throwable t) {
+ failure.set(t);
+ }
+ }
+ }, name);
+ }
+
+ void joinOrFail() throws Exception {
+ thread.join(WAIT_MILLIS);
+ assertFalse(thread.isAlive(), thread.getName() + " did not finish within " + WAIT_MILLIS + " ms");
+ final Throwable thrown = failure.get();
+ if (thrown instanceof Exception) {
+ throw (Exception) thrown;
+ }
+ if (thrown != null) {
+ throw new AssertionError(thrown);
+ }
+ }
+ }
+
+ private static Background start(String name, Execution execution) {
+ final Background background = new Background(name, execution);
+ background.thread.start();
+ return background;
+ }
+
+ /**
+ * An entry read that has not come back in two minutes is stuck, while a count or the delete
+ * that empties a tree before an import legitimately takes longer than anything can guess - so
+ * the bulk class stays unbounded until a deployment says otherwise.
+ */
+ @Test
+ public void testDefaultsBoundAnOperationAndLeaveBulkAlone() throws Exception {
+ assertEquals(StatementBound.OPERATION.seconds(), 120);
+ assertEquals(StatementBound.BULK.seconds(), 0);
+ }
+
+ @Test
+ public void testEachClassIsConfiguredByItsOwnProperty() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ assertEquals(StatementBound.OPERATION.seconds(), 7);
+ assertEquals(StatementBound.BULK.seconds(), 0, "the bulk class followed the operation one");
+
+ System.setProperty(StatementBound.BULK.property, "900");
+ assertEquals(StatementBound.BULK.seconds(), 900);
+ assertEquals(StatementBound.OPERATION.seconds(), 7);
+ }
+
+ @Test
+ public void testAValueThatIsNoBoundLeavesTheStatementUnbounded() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "0");
+ assertEquals(StatementBound.OPERATION.seconds(), 0);
+
+ System.setProperty(StatementBound.OPERATION.property, "-1");
+ assertEquals(StatementBound.OPERATION.seconds(), 0);
+ }
+
+ /**
+ * A value that is not a number is not a way to switch the bound off: it is ignored in favour
+ * of the default, as {@code Integer.getInteger()} has it, so a typo leaves the class bounded
+ * rather than silently unbounding it.
+ */
+ @Test
+ public void testAValueThatIsNotANumberFallsBackToTheDefault() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "two minutes");
+ assertEquals(StatementBound.OPERATION.seconds(), 120);
+
+ // from a bound that took, so that the fallback is what the assertion below can be seeing:
+ // the default of this class is 0, which is also what a value read as a number would give
+ System.setProperty(StatementBound.BULK.property, "900");
+ assertEquals(StatementBound.BULK.seconds(), 900);
+ System.setProperty(StatementBound.BULK.property, "as long as it takes");
+ assertEquals(StatementBound.BULK.seconds(), 0);
+ }
+
+ /**
+ * A bound larger than the second layer can hold is taken down to what it can hold, and stays a
+ * bound: that layer is a socket read timeout, which is milliseconds of an {@code int}, so
+ * {@code Integer.MAX_VALUE} - the usual "no bound" idiom - has no value of it to be given, and
+ * a negative one is a call every driver refuses, leaving the connection carrying whatever the
+ * statement before it armed. {@code 0} is what says "no bound" here, and the ceiling is not it.
+ */
+ @Test
+ public void testABoundLargerThanTheBackstopCanHoldIsTakenDownToIt() throws Exception {
+ assertEquals(JDBCStorage.clampSeconds(Integer.MAX_VALUE), JDBCStorage.MAX_BOUND_SECONDS);
+ System.setProperty(StatementBound.OPERATION.property, String.valueOf(Integer.MAX_VALUE));
+ assertEquals(StatementBound.OPERATION.seconds(), JDBCStorage.MAX_BOUND_SECONDS);
+
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+
+ storage.execute(statement);
+
+ verify(statement).setQueryTimeout(JDBCStorage.MAX_BOUND_SECONDS);
+ // and what the second layer is armed with is still a positive int of milliseconds, which is
+ // the whole of what the ceiling is for: a negative one is the call a driver refuses
+ verify(con).setNetworkTimeout(any(Executor.class),
+ eq((JDBCStorage.MAX_BOUND_SECONDS + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ }
+
+ @Test
+ public void testTheBoundReachesTheStatement() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+
+ storage.execute(statement);
+
+ verify(statement).setQueryTimeout(7);
+ }
+
+ /** An unbounded class costs no call of its own: a fresh statement is unbounded already. */
+ @Test
+ public void testAnUnboundedClassSetsNothing() throws Exception {
+ final PreparedStatement statement = mock(PreparedStatement.class);
+
+ storage.execute(statement, StatementBound.BULK);
+
+ verify(statement, never()).setQueryTimeout(anyInt());
+ }
+
+ /**
+ * Behind the cancel is a socket read timeout, for the databases that do not act on a cancel:
+ * it is armed for the statement and put back once nothing is running on the connection any
+ * more. What a connection carrying several statements at once does with it is pinned by the
+ * three tests below.
+ */
+ @Test
+ public void testTheBackstopIsArmedAndPutBack() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0); // no bound of its own
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+
+ storage.execute(statement);
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * The backstop only ever tightens. A read timeout a deployment gave its connections is the
+ * bound it asked for, and this one - deliberately the looser of the two, so that the cancel
+ * has room to arrive first - must not stand in for it while a statement runs.
+ */
+ @Test
+ public void testTheBackstopDoesNotLoosenATighterBound() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(5000); // tighter than 7s plus the margin
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+
+ storage.execute(statement);
+
+ verify(con, never()).setNetworkTimeout(any(Executor.class), anyInt());
+ }
+
+ /**
+ * A statement of a class that carries no bound takes the backstop off the connection for as
+ * long as it runs. The socket read timeout is a property of the connection, and an importer
+ * writes to a single one from every phase-one worker and every phase-two task, so the bulk
+ * {@code delete from} that empties a tree would otherwise be cut at the bound of an entry read
+ * happening to run beside it - and cut without ever naming a property, since a statement of an
+ * unbounded class has none to name. Two threads, because that is how the two meet.
+ */
+ @Test
+ public void testAnUnboundedStatementTakesTheBackstopOffWhileItRuns() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "0");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final CountDownLatch operationRunning = new CountDownLatch(1);
+ final CountDownLatch operationMayFinish = new CountDownLatch(1);
+ final CountDownLatch bulkRunning = new CountDownLatch(1);
+ final CountDownLatch bulkMayFinish = new CountDownLatch(1);
+ final PreparedStatement operation = lingering(con, operationRunning, operationMayFinish);
+ final PreparedStatement bulk = lingering(con, bulkRunning, bulkMayFinish);
+
+ final Background entryRead = start("entry-read", () -> storage.execute(operation));
+ awaitOrFail(operationRunning, "the entry read never started");
+ final Background clearTree = start("clear-tree", () -> storage.execute(bulk, StatementBound.BULK));
+ awaitOrFail(bulkRunning, "the bulk statement never started");
+ bulkMayFinish.countDown();
+ clearTree.joinOrFail();
+ operationMayFinish.countDown();
+ entryRead.joinOrFail();
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0)); // the bulk statement takes it off
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0)); // and the entry read is through
+ }
+
+ /**
+ * The backstop belongs to the connection, not to the statement that armed it: the first
+ * statement to finish must not take it away from the statements still running there.
+ */
+ @Test
+ public void testTheBackstopOutlastsTheStatementThatArmedIt() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final CountDownLatch running = new CountDownLatch(1);
+ final CountDownLatch mayFinish = new CountDownLatch(1);
+ final PreparedStatement lingering = lingering(con, running, mayFinish);
+ final PreparedStatement passing = mock(PreparedStatement.class);
+ when(passing.getConnection()).thenReturn(con);
+ when(passing.executeUpdate()).thenReturn(1);
+
+ final Background outliving = start("outliving", () -> storage.execute(lingering));
+ awaitOrFail(running, "the statement that arms the backstop never started");
+ storage.execute(passing); // joins that connection and is through while the other one runs
+
+ verify(con, never()).setNetworkTimeout(any(Executor.class), eq(0));
+ mayFinish.countDown();
+ outliving.joinOrFail();
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * With bounds of two classes in flight on one connection, the value armed is the loosest of
+ * them: a socket read timeout is shared by everything running on the connection, so tightening
+ * it to the bound of an entry read would cut the bulk statement beside it long before the bound
+ * that statement was actually given.
+ */
+ @Test
+ public void testTheBackstopFollowsTheLoosestBoundInFlight() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "100");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final CountDownLatch bulkRunning = new CountDownLatch(1);
+ final CountDownLatch bulkMayFinish = new CountDownLatch(1);
+ final PreparedStatement bulk = lingering(con, bulkRunning, bulkMayFinish);
+ final PreparedStatement operation = mock(PreparedStatement.class);
+ when(operation.getConnection()).thenReturn(con);
+ when(operation.executeUpdate()).thenReturn(1);
+
+ final Background count = start("count", () -> storage.execute(bulk, StatementBound.BULK));
+ awaitOrFail(bulkRunning, "the bulk statement never started");
+ storage.execute(operation); // an entry read of another thread, with a tighter bound
+ bulkMayFinish.countDown();
+ count.joinOrFail();
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((100 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ verify(con, never()).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+
+ }
+
+ /**
+ * The other order, which is the one that moves the backstop while a statement is running: a
+ * bound looser than what is armed re-arms the connection to its own value, and the tighter
+ * statement left behind gets its bound back the moment the looser one is through.
+ */
+ @Test
+ public void testALooserBoundRearmsTheBackstopAndTheTighterOneGetsItBack() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "100");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final CountDownLatch operationRunning = new CountDownLatch(1);
+ final CountDownLatch operationMayFinish = new CountDownLatch(1);
+ final CountDownLatch bulkRunning = new CountDownLatch(1);
+ final CountDownLatch bulkMayFinish = new CountDownLatch(1);
+ final PreparedStatement operation = lingering(con, operationRunning, operationMayFinish);
+ final PreparedStatement bulk = lingering(con, bulkRunning, bulkMayFinish);
+
+ final Background entryRead = start("entry-read", () -> storage.execute(operation));
+ awaitOrFail(operationRunning, "the entry read never started");
+ final Background count = start("count", () -> storage.execute(bulk, StatementBound.BULK));
+ awaitOrFail(bulkRunning, "the bulk statement never started");
+ bulkMayFinish.countDown();
+ count.joinOrFail();
+ operationMayFinish.countDown();
+ entryRead.joinOrFail();
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((100 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * A failure that arrives before the bound is the caller's to classify and must reach it as it
+ * stands: a lock wait reported in class 40 is the conflict {@code JDBCStorage.write()} replays,
+ * and wrapping it would take it out of that class.
+ */
+ @Test
+ public void testAFailureInsideTheBoundIsPassedThrough() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "60");
+ final SQLException conflict = new SQLException("lock wait timeout exceeded", "40001", 1205);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenThrow(conflict);
+
+ try {
+ storage.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLException e) {
+ assertSame(e, conflict);
+ }
+ }
+
+ /**
+ * A failure that arrives at the bound names the property that produced it - every driver
+ * reports a cancelled statement differently, and none of them knows why it was cancelled - and
+ * still carries the SQL state and the error number of the failure it replaces.
+ */
+ @Test
+ public void testAFailureAtTheBoundNamesTheProperty() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ Thread.sleep(1100); // the driver cancelled it at the bound this test set
+ throw new SQLException("canceling statement due to user request", "57014", 0);
+ }
+ });
+
+ try {
+ storage.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLTimeoutException e) {
+ assertEquals(e.getSQLState(), "57014");
+ assertTrue(e.getMessage().contains(StatementBound.OPERATION.property), e.getMessage());
+ assertEquals(((SQLException) e.getCause()).getSQLState(), "57014");
+ }
+ }
+
+ /**
+ * A driver reporting the cancel a few milliseconds before the bound is arithmetically due -
+ * its timer is kept in whole seconds, while this classification is measured to the millisecond
+ * - is still the bound arriving, and the failure has to name the property all the same.
+ * Without the slack under it, the one failure this classification exists to name reached the
+ * caller as a bare 57014 or ORA-01013, which no caller can tell from a cancel of an operator.
+ */
+ @Test
+ public void testAFailureJustUnderTheBoundStillNamesTheProperty() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final SteppedClockStorage clocked = new SteppedClockStorage();
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ // a literal, deliberately not derived from CLOCK_SLACK_MILLIS: a test computing its
+ // own input from the constant it pins follows that constant to zero and pins nothing
+ clocked.millis = 875; // 125 ms under the bound, inside a slack of 250
+ throw new SQLException("canceling statement due to user request", "57014", 0);
+ }
+ });
+
+ try {
+ clocked.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLTimeoutException e) {
+ assertTrue(e.getMessage().contains(StatementBound.OPERATION.property), e.getMessage());
+ // and the time it names is the one measured, so that this cannot pass on a clock that
+ // simply ran past the bound while the assertion above looked only at the message
+ assertTrue(e.getMessage().contains("875 ms"), e.getMessage());
+ }
+ }
+
+ /**
+ * The other side of that slack, which is what keeps it a slack rather than a second bound: a
+ * failure further under the bound than a driver's whole-second timer could account for is a
+ * failure of its own, and the caller has to see it as one.
+ */
+ @Test
+ public void testAFailureFurtherUnderTheBoundIsStillPassedThrough() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final SteppedClockStorage clocked = new SteppedClockStorage();
+ final SQLException conflict = new SQLException("lock wait timeout exceeded", "40001", 1205);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ clocked.millis = 749; // a millisecond further out than the slack of 250 reaches
+ throw conflict;
+ }
+ });
+
+ try {
+ clocked.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLException e) {
+ assertSame(e, conflict);
+ }
+ }
+
+ /**
+ * And the edge of that slack belongs to the bound. The two cases above sit either side of it,
+ * so both of them pass whether the comparison is {@code <} or {@code <=} - the point where the
+ * two differ is exactly a slack under the bound, and that is what this pins.
+ */
+ @Test
+ public void testAFailureExactlyASlackUnderTheBoundIsStillTheBound() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final SteppedClockStorage clocked = new SteppedClockStorage();
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ // a literal for the reason the case above gives: derived from CLOCK_SLACK_MILLIS it
+ // would follow that constant wherever it went and pin nothing
+ clocked.millis = 750; // the bound of a second, less a slack of 250
+ throw new SQLException("canceling statement due to user request", "57014", 0);
+ }
+ });
+
+ try {
+ clocked.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLTimeoutException e) {
+ assertTrue(e.getMessage().contains(StatementBound.OPERATION.property), e.getMessage());
+ assertTrue(e.getMessage().contains("750 ms"), e.getMessage());
+ }
+ }
+
+ /**
+ * A statement that neither layer bounded reaches its caller exactly as it is. Asking for the
+ * second layer is not having it: a driver may have no network timeout at all, a connection may
+ * refuse the call, one may already carry a timeout of a deployment's own, and a statement of an
+ * unbounded class running beside this one takes the backstop off outright. A catalog lookup
+ * takes no query timeout by construction, so with the backstop unarmed nothing ends its wait -
+ * and the failure that finally does is the driver's own. Reported as the bound, it sent an
+ * operator to raise a property that had bounded nothing about the wait they had just watched.
+ */
+ @Test
+ public void testAFailureOfAStatementNeitherLayerBoundedIsPassedThrough() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final SteppedClockStorage clocked = new SteppedClockStorage(); // its own, for its own latch
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ doThrow(new SQLFeatureNotSupportedException("no network timeout"))
+ .when(con).setNetworkTimeout(any(Executor.class), anyInt());
+ final SQLException reset = new SQLException("connection reset by peer", "08006", 0);
+
+ try {
+ clocked.bounded(con, StatementBound.OPERATION, () -> {
+ clocked.millis = 600000; // ten minutes on a metadata lock, with nothing to end it
+ throw reset;
+ });
+ fail("the failure of the lookup must reach the caller");
+ } catch (SQLException e) {
+ assertSame(e, reset, "a statement neither layer bounded was reported as having reached a bound");
+ }
+ }
+
+ /**
+ * The failure reports the time the statement really took rather than the bound it reached.
+ * A cancel that is armed is not a cancel that is acted upon - a session blocked in a row-lock
+ * enqueue on oracle does not process the break its driver sends - and the wait then ends at
+ * the socket read timeout behind it, a margin later than the property that armed it. Reported
+ * as the property alone, the message put a time next to a clock that disagreed with it.
+ */
+ @Test
+ public void testAFailureAtTheBoundReportsTheTimeItReallyTook() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeUpdate()).thenAnswer(new Answer<Integer>() {
+ @Override
+ public Integer answer(InvocationOnMock invocation) throws Throwable {
+ Thread.sleep(1500); // the cancel was armed at 1 s and the database did not act on it
+ throw new SQLException("connection reset by peer", "08006", 0);
+ }
+ });
+
+ try {
+ storage.execute(statement);
+ fail("the failure of the statement must reach the caller");
+ } catch (SQLTimeoutException e) {
+ final Matcher took = Pattern.compile("took (\\d+) ms").matcher(e.getMessage());
+ assertTrue(took.find(), e.getMessage());
+ assertTrue(Long.parseLong(took.group(1)) >= 1500, e.getMessage());
+ }
+ }
+
+ /**
+ * The rows are read while the bound is still armed. A driver hands them over as they are asked
+ * for - oracle prefetches ten at a time, mssql buffers adaptively - so a drain that happened
+ * after the bound was released would be a wait with nothing bounding it, which is the hang
+ * #877 is about rather than a detail of where the call sits.
+ */
+ @Test
+ public void testTheRowsAreReadWhileTheBoundIsStillArmed() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final ResultSet rows = mock(ResultSet.class);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+ when(statement.executeQuery()).thenReturn(rows);
+
+ assertEquals(storage.executeResultSet(statement, ResultSet::next), Boolean.FALSE);
+
+ final InOrder inOrder = inOrder(con, rows);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(rows).next();
+ inOrder.verify(rows).close(); // the rows are done with before the backstop goes back
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /** A transfer of rows cut at the bound names the property that cut it, as an execution does. */
+ @Test
+ public void testAFailureWhileTheRowsAreReadIsMeasuredAgainstTheBound() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final ResultSet rows = mock(ResultSet.class);
+ when(rows.next()).thenAnswer(new Answer<Boolean>() {
+ @Override
+ public Boolean answer(InvocationOnMock invocation) throws Throwable {
+ Thread.sleep(1100); // the driver cancelled the transfer at the bound this test set
+ throw new SQLException("canceling statement due to user request", "57014", 0);
+ }
+ });
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(rows);
+
+ try {
+ storage.executeResultSet(statement, ResultSet::next);
+ fail("the failure of the transfer must reach the caller");
+ } catch (SQLTimeoutException e) {
+ assertTrue(e.getMessage().contains(StatementBound.OPERATION.property), e.getMessage());
+ assertEquals(e.getSQLState(), "57014");
+ }
+ }
+
+ /**
+ * A driver is allowed to have no query timeout at all, and this backend takes whatever URL a
+ * deployment configures. Such a driver has to keep working, with the socket read timeout as
+ * its whole bound, rather than fail every statement it is given.
+ */
+ @Test
+ public void testADriverWithoutAQueryTimeoutKeepsWorkingUnderTheBackstop() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+ doThrow(new SQLFeatureNotSupportedException("no query timeout")).when(statement).setQueryTimeout(anyInt());
+ when(statement.executeUpdate()).thenReturn(1);
+
+ assertEquals(storage.execute(statement), 1);
+
+ verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ }
+
+ /**
+ * The catalog lookups of {@code openTree()} are bounded too. {@code DatabaseMetaData} takes no
+ * query timeout, so the socket read timeout behind the cancel is the only layer they can be
+ * given - and they run once per tree on every open of a backend, behind the same locks as the
+ * {@code create table} they guard.
+ */
+ @Test
+ public void testACatalogLookupIsBoundedByTheBackstopAlone() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+
+ assertEquals(storage.bounded(con, StatementBound.OPERATION, () -> "asked the catalog"), "asked the catalog");
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * Which class a batch of a cursor belongs to follows what the database has to do to answer it.
+ * {@code positionToLastKey()} has no key to seek on, so it is an {@code order by k desc} over
+ * the whole table - a scan and a sort of it on mssql, where {@code k} cannot be an index key -
+ * and every open of a backend runs it once per base DN through
+ * {@code EntryContainer.getHighestEntryID()}, outside the try/catch of
+ * {@code BackendImpl.openBackend()}. Bounding that as an entry read would turn a large backend
+ * that opens slowly into one that does not open at all, while the batches the cursor walks
+ * along its index stay operations and keep the bound of one.
+ */
+ @Test
+ public void testTheScanBehindTheHighestEntryIdIsBulkAndTheBatchesOfACursorAreNot() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "0");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(mock(ResultSet.class));
+ final Connection parent = mock(Connection.class);
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+ final JDBCStorage.CursorImpl cursor = storage.new CursorImpl(true, new CachedConnection("jdbc:mock", parent),
+ new TreeName("dc=example,dc=com", "id2entry"), StatementBound.OPERATION);
+
+ cursor.positionToLastKey();
+ verify(statement, never()).setQueryTimeout(anyInt());
+
+ cursor.next();
+ verify(statement).setQueryTimeout(7);
+ }
+
+ /**
+ * A cursor opened for a walk of a whole tree takes bulk batches, however ordinary the statement
+ * looks: nobody is waiting on that walk, and on mssql it is not even a walk along an index -
+ * {@code k} is a {@code varbinary(max)} there, which cannot be an index key, so every batch is
+ * a scan and a sort of the whole table. This is the class an export, a verify, a rebuild and
+ * the load of a tree at open ask for through {@code ReadableTransaction.openBulkCursor()},
+ * while the cursor of a search keeps the bound of an operation.
+ */
+ @Test
+ public void testTheBatchesOfABulkCursorAreBulkAndThoseOfASearchAreNot() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "0");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(mock(ResultSet.class));
+ final Connection parent = mock(Connection.class);
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+ final JDBCStorage.ReadableTransactionImpl txn =
+ storage.new ReadableTransactionImpl(new CachedConnection("jdbc:mock", parent));
+ final TreeName tree = new TreeName("dc=example,dc=com", "id2entry");
+
+ txn.openBulkCursor(tree).next();
+ verify(statement, never()).setQueryTimeout(anyInt());
+
+ txn.openCursor(tree).next();
+ verify(statement).setQueryTimeout(7);
+ }
+
+ /**
+ * Every statement an import issues is bulk, by the class of the transactions it works through:
+ * phase one writes the trees through {@code put()}, phase two reads them back through
+ * {@code read()} and walks them through {@code openCursor()}, and no client is waiting on any
+ * of it. An upsert of an online import blocked by an LDAP write on the same table would
+ * otherwise sit until the bound of an entry read and then fail the whole import - {@code h} is
+ * the primary key on every dialect, and the default lock wait is forever on three of the four.
+ */
+ @Test
+ public void testEveryStatementOfAnImportIsBulk() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "0");
+ final Connection parent = mock(Connection.class);
+ // a read timeout of a deployment's own, and deliberately not zero: restoring it and taking
+ // the backstop off are one and the same call when what came before was zero, and the
+ // assertions below would then hold whichever of the two the code did
+ when(parent.getNetworkTimeout()).thenReturn(90000);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ // the statement reports the connection it runs on, as CachedConnection.prepareStatement()
+ // has it: without this the import would be measured against the first layer alone, and the
+ // second one - the layer that decides whether an import can hang on a peer that stopped
+ // answering - would never be entered at all
+ when(statement.getConnection()).thenReturn(parent);
+ when(statement.executeQuery()).thenReturn(mock(ResultSet.class));
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+ storage.accessMode = AccessMode.READ_WRITE; // an import has the storage open for writing
+ final JDBCStorage.ImporterImpl importer =
+ storage.new ImporterImpl(new CachedConnection("jdbc:mock", parent), true);
+ final TreeName tree = new TreeName("dc=example,dc=com", "id2entry");
+
+ // an entry read of a client arms the backstop on the very connection the import writes to,
+ // which is the shape of an online import: one connection, statements of both classes on it
+ final CountDownLatch running = new CountDownLatch(1);
+ final CountDownLatch mayFinish = new CountDownLatch(1);
+ final PreparedStatement operation = lingering(parent, running, mayFinish);
+ final Background entryRead = start("entry-read", () -> storage.execute(operation));
+ awaitOrFail(running, "the entry read never started");
+ // atLeastOnce, not the implicit times(1) of a bare verify: every statement of the import
+ // below takes the backstop off and its release arms it again while the entry read is still
+ // in flight, so this holds by where it stands in the method rather than by what it pins
+ verify(parent, atLeastOnce())
+ .setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+
+ importer.openCursor(tree).next();
+ importer.read(tree, ByteString.valueOfUtf8("key"));
+ importer.put(tree, ByteString.valueOfUtf8("key"), ByteString.valueOfUtf8("value"));
+
+ verify(statement, never()).setQueryTimeout(anyInt());
+ // and none of them runs under the socket read timeout of the entry read beside it either:
+ // while that read is still in flight, the import takes the backstop off the connection,
+ // which is what putting the connection's own value back looks like
+ verify(parent, atLeastOnce()).setNetworkTimeout(any(Executor.class), eq(90000));
+ // and never a zero: nothing here has a zero to put back, so a run that reached this state
+ // by restoring one would be a run that read the previous value of another connection
+ verify(parent, never()).setNetworkTimeout(any(Executor.class), eq(0));
+
+ mayFinish.countDown();
+ entryRead.joinOrFail();
+ }
+
+ /**
+ * A statement bounded by the socket read timeout alone is measured against what that layer
+ * really allows it - its bound plus the margin the layer carries - rather than against the
+ * property: nothing cuts a catalog lookup at the bound itself, so a connection reset arriving
+ * just after it is the caller's failure to see, not a query timeout that never happened.
+ */
+ @Test
+ public void testAFailureBeforeTheBackstopOfACatalogLookupIsPassedThrough() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "1");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ final SQLException reset = new SQLException("connection reset by peer", "08006", 0);
+
+ try {
+ storage.bounded(con, StatementBound.OPERATION, () -> {
+ sleep(1100); // past the property, well inside the margin of the layer behind it
+ throw reset;
+ });
+ fail("the failure of the lookup must reach the caller");
+ } catch (SQLException e) {
+ assertSame(e, reset);
+ }
+ }
+
+ /**
+ * A driver with no network timeout at all is asked once and then left alone: it says so with
+ * {@code SQLFeatureNotSupportedException}, and asking it again costs a throw on every statement
+ * for the life of the storage. Its own storage here, since that is the scope of the latch.
+ */
+ @Test
+ public void testADriverWithoutANetworkTimeoutIsNotAskedAgain() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final JDBCStorage isolated = new JDBCStorage(mockCfg(JDBCBackendCfg.class), null);
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ doThrow(new SQLFeatureNotSupportedException("no network timeout"))
+ .when(con).setNetworkTimeout(any(Executor.class), anyInt());
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+ when(statement.executeUpdate()).thenReturn(1);
+
+ assertEquals(isolated.execute(statement), 1);
+ assertEquals(isolated.execute(statement), 1);
+
+ verify(con, times(1)).setNetworkTimeout(any(Executor.class), anyInt());
+ }
+
+ /**
+ * A connection that failed the call says nothing about the driver - it may be the very one
+ * that reached this timeout - so the next statement is armed as usual. The two causes share a
+ * catch and must not share a verdict: taking one for the other silences the backstop of a
+ * whole storage on a single dying connection.
+ */
+ @Test
+ public void testAConnectionThatFailedTheBackstopDoesNotSpeakForTheDriver() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ final Connection con = mock(Connection.class);
+ when(con.getNetworkTimeout()).thenReturn(0);
+ doThrow(new SQLException("the connection is closed", "08003", 0))
+ .when(con).setNetworkTimeout(any(Executor.class), anyInt());
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.getConnection()).thenReturn(con);
+ when(statement.executeUpdate()).thenReturn(1);
+
+ assertEquals(storage.execute(statement), 1);
+ assertEquals(storage.execute(statement), 1);
+
+ verify(con, times(2)).setNetworkTimeout(any(Executor.class),
+ eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ }
+
+ /**
+ * The statistics refresh after an import runs under a bound of its own - it takes as long as a
+ * scan of the table it describes, which no class of {@link StatementBound} can be asked to
+ * allow - and under both layers of it. The second one is the reason: on oracle this statement
+ * is {@code dbms_stats.gather_table_stats}, the engine whose session does not act on the break
+ * its driver sends, and it runs at the very end of a successful import, where a cancel that
+ * never arrives would park the import with its data already committed.
+ */
+ @Test
+ public void testTheStatisticsRefreshRunsUnderItsOwnBoundAndTheBackstop() throws Exception {
+ System.setProperty(JDBCStorage.STATISTICS_TIMEOUT_PROPERTY, "60");
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ final Connection con = mock(oracleConnection.class); // the dialect is read off the connection
+ when(con.getNetworkTimeout()).thenReturn(0);
+ when(con.prepareStatement(anyString())).thenReturn(statement);
+
+ assertTrue(storage.updateTableStatistics(con, singletonList(new TreeName("dc=example,dc=com", "id2entry"))));
+
+ verify(statement).setQueryTimeout(60);
+ final InOrder inOrder = inOrder(con, statement);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((60 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(statement).execute();
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(0));
+ }
+
+ /**
+ * A connection whose driver refuses the call mid-flight is given back what it carried before.
+ * The entry holding that value is dropped as soon as the last statement on the connection is
+ * through, so a backstop left armed goes back to the pool as the connection's own read timeout
+ * - and the next borrower, which only ever tightens, reads it as the value of a deployment and
+ * keeps it from then on, cutting a statement of an unbounded class at a bound it never had.
+ */
+ @Test
+ public void testAConnectionThatFailedTheBackstopIsGivenBackWhatItCarried() throws Exception {
+ System.setProperty(StatementBound.OPERATION.property, "7");
+ System.setProperty(StatementBound.BULK.property, "100");
+ final Connection con = mock(Connection.class);
+ // a read timeout of a deployment's own, and looser than either bound in flight below: a
+ // tighter one is what the backstop declines to loosen, and it would put that value back
+ // instead of ever reaching the call that fails here
+ when(con.getNetworkTimeout()).thenReturn(200000);
+ // the arming of the tighter bound goes through, and the re-arm of the looser one does not
+ doThrow(new SQLException("the connection is closed", "08003", 0)).when(con)
+ .setNetworkTimeout(any(Executor.class), eq((100 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ final CountDownLatch running = new CountDownLatch(1);
+ final CountDownLatch mayFinish = new CountDownLatch(1);
+ final PreparedStatement operation = lingering(con, running, mayFinish);
+ final PreparedStatement bulk = mock(PreparedStatement.class);
+ when(bulk.getConnection()).thenReturn(con);
+ when(bulk.executeUpdate()).thenReturn(1);
+
+ final Background entryRead = start("entry-read", () -> storage.execute(operation));
+ awaitOrFail(running, "the entry read never started");
+ assertEquals(storage.execute(bulk, StatementBound.BULK), 1); // its re-arm is what fails
+ mayFinish.countDown();
+ entryRead.joinOrFail();
+
+ final InOrder inOrder = inOrder(con);
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq((7 + JDBCStorage.BACKSTOP_MARGIN_SECONDS) * 1000));
+ inOrder.verify(con).setNetworkTimeout(any(Executor.class), eq(200000));
+ }
+
+ /**
+ * The connection an import would have held goes back to the pool when the importer cannot be
+ * built on it. That is a designed path rather than an accident: an import of a read-only storage
+ * throws {@code ReadOnlyStorageException} where the importer is built, and the connection
+ * borrowed for the import - the one it keeps for its whole duration - was leaving the pool for
+ * good there, with the transaction it had already begun.
+ */
+ @Test
+ public void testStartImportGivesTheConnectionBackWhenTheImporterCannotBeBuilt() throws Exception {
+ final Connection con = mock(Connection.class);
+ final JDBCStorage readOnly = new JDBCStorage(mockCfg(JDBCBackendCfg.class), null) {
+ @Override
+ Connection getConnection(boolean trusted) {
+ return con;
+ }
+
+ @Override
+ public StorageStatus getStorageStatus() {
+ return StorageStatus.working(); // open already, so that startImport() borrows and no more
+ }
+ };
+ readOnly.accessMode = AccessMode.READ_ONLY;
+
+ try {
+ readOnly.startImport();
+ fail("an import of a read-only storage must not be handed an importer");
+ } catch (ReadOnlyStorageException expected) {
+ // the designed path this test is about
+ }
+
+ verify(con).close();
+ }
+
+ /**
+ * And the storage goes back with the connection where this method is what opened it:
+ * {@code ImporterImpl.close()} is the only thing that closes a storage an import opened, so a
+ * failure between the open and the importer that would have held it leaves it open for good.
+ * The borrow of the connection was already covered that way; the build of the importer was not.
+ * <p>
+ * What fails the build here is a storage that is not writeable - the one failure of the
+ * importer's constructor a test can produce from outside it, and it takes an open that leaves
+ * the storage read-only to get there. What it stands for is any {@code Error} out of that
+ * constructor, which is what the {@code finally} is for.
+ */
+ @Test
+ public void testStartImportClosesTheStorageItOpenedWhenTheImporterCannotBeBuilt() throws Exception {
+ final Connection con = mock(Connection.class);
+ final AtomicInteger opens = new AtomicInteger();
+ final AtomicInteger closes = new AtomicInteger();
+ final JDBCStorage notOpen = new JDBCStorage(mockCfg(JDBCBackendCfg.class), null) {
+ @Override
+ Connection getConnection(boolean trusted) {
+ return con;
+ }
+
+ @Override
+ public StorageStatus getStorageStatus() {
+ return StorageStatus.lockedDown(LocalizableMessage.raw("closed")); // so startImport() opens it
+ }
+
+ @Override
+ public void open(AccessMode accessMode) {
+ opens.incrementAndGet(); // and leaves this storage read-only, so that the build below fails
+ }
+
+ @Override
+ public void close() {
+ closes.incrementAndGet();
+ }
+ };
+
+ try {
+ notOpen.startImport();
+ fail("an import that cannot be given an importer must not report one");
+ } catch (ReadOnlyStorageException expected) {
+ // the build failing after this method opened the storage, which is the path under test
+ }
+
+ assertEquals(opens.get(), 1, "the storage was not opened by startImport(), so nothing was owed back");
+ verify(con).close();
+ assertEquals(closes.get(), 1, "the storage this method opened was left open");
+ }
+
+ /**
+ * A row whose {@code v} is null is a row that exists, and reading it has to fail rather than
+ * report the key as absent - which is what {@code read()} of the same row does. Reading the
+ * rows inside the bound had turned the value into a raw {@code byte[]} on the way out of the
+ * handler, and null then stood in for both.
+ */
+ @Test
+ public void testARowWithoutAValueFailsRatherThanReportingTheKeyAsAbsent() throws Exception {
+ final TreeName treeName = new TreeName("dc=example,dc=com", "id2entry");
+ final ResultSet rows = mock(ResultSet.class);
+ when(rows.next()).thenReturn(true);
+ when(rows.getBytes("v")).thenReturn(null); // the schema allows it, however this backend writes
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(rows);
+ final Connection parent = mock(Connection.class);
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+ final JDBCStorage.CursorImpl cursor = storage.new CursorImpl(true, new CachedConnection("jdbc:mock", parent),
+ treeName, StatementBound.OPERATION);
+
+ try {
+ cursor.positionToKey(ByteString.valueOfUtf8("key"));
+ fail("a row whose value is null must not be read as a key that is not there");
+ } catch (StorageRuntimeException expected) {
+ // the failure the production path names, not whatever null happens to reach first: a bare
+ // NullPointerException out of ByteString.wrap is satisfied by any unrelated one later
+ // introduced into positionToKey, and it says nothing about which table holds the row
+ assertTrue(expected.getMessage().contains("no value"), expected.getMessage());
+ assertTrue(expected.getMessage().contains(storage.getTableName(treeName)), expected.getMessage());
+ }
+ }
+
+ /**
+ * And so does a batch of a cursor, which is the third reader of a value and the one that would
+ * fail furthest from the row: a batch is buffered whole and unwrapped a row at a time
+ * afterwards, so left unchecked it fails from {@code advanceFromBuffer()} - outside the bound
+ * and outside the {@code catch} of the batch that read it.
+ */
+ @Test
+ public void testABatchWithARowWithoutAValueFailsTheSameWay() throws Exception {
+ final TreeName treeName = new TreeName("dc=example,dc=com", "id2entry");
+ final ResultSet rows = mock(ResultSet.class);
+ when(rows.next()).thenReturn(true, false);
+ when(rows.getBytes(1)).thenReturn(ByteString.valueOfUtf8("key").toByteArray());
+ when(rows.getBytes(2)).thenReturn(null);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(rows);
+ final Connection parent = mock(Connection.class);
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+ final JDBCStorage.CursorImpl cursor = storage.new CursorImpl(true, new CachedConnection("jdbc:mock", parent),
+ treeName, StatementBound.OPERATION);
+
+ try {
+ cursor.next();
+ fail("a batch holding a row whose value is null must not hand that row out");
+ } catch (StorageRuntimeException expected) {
+ assertTrue(expected.getMessage().contains("no value"), expected.getMessage());
+ assertTrue(expected.getMessage().contains(storage.getTableName(treeName)), expected.getMessage());
+ }
+ }
+
+ /**
+ * And a read of the same row fails the same way, which is the whole of what {@code null} means
+ * here: the two answer with it for one reason only, and a row that exists is not that reason.
+ */
+ @Test
+ public void testAReadOfARowWithoutAValueFailsTheSameWay() throws Exception {
+ final TreeName treeName = new TreeName("dc=example,dc=com", "id2entry");
+ final ResultSet rows = mock(ResultSet.class);
+ when(rows.next()).thenReturn(true);
+ when(rows.getBytes("v")).thenReturn(null);
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ when(statement.executeQuery()).thenReturn(rows);
+ final Connection parent = mock(Connection.class);
+ when(parent.prepareStatement(anyString())).thenReturn(statement);
+
+ try {
+ storage.new ReadableTransactionImpl(new CachedConnection("jdbc:mock", parent))
+ .read(treeName, ByteString.valueOfUtf8("key"));
+ fail("a row whose value is null must not be read as a key that is not there");
+ } catch (StorageRuntimeException expected) {
+ assertTrue(expected.getMessage().contains("no value"), expected.getMessage());
+ }
+ }
+
+ // JDBCStorage.dialectOf() reads the engine off the class name of the connection, so a mock of
+ // this interface is an oracle connection as far as the storage is concerned - which is the
+ // whole reason for the lower case name here.
+ private interface oracleConnection extends Connection {}
+}
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 cd8057e..3f2f14e 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
@@ -55,6 +55,7 @@
import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
import static org.mockito.Mockito.when;
+import static org.opends.server.util.StaticUtils.stackTraceToSingleLineString;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotEquals;
@@ -442,6 +443,199 @@
}
/**
+ * A statement of this backend has to end even when another session holds what it needs: a row
+ * locked by a transaction that never commits used to park the worker thread that issued the
+ * write for good, with nothing in the log to say so (#877).
+ */
+ @Test(timeOut = 600000)
+ public void testWriteBlockedByAnotherSessionGivesUpAtItsBound() throws Exception {
+ assertBoundedWhileRowsAreLocked("testStatementBound", JDBCStorage.StatementBound.OPERATION,
+ new BlockedOperation() {
+ @Override
+ public void run(JDBCStorage storage, TreeName tree) throws Exception {
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.put(tree, key(1), value(2));
+ }
+ });
+ }
+ });
+ }
+
+ /**
+ * The bulk class keeps a bound of its own: a count or the delete that empties a tree before an
+ * import legitimately takes minutes, so it must not be cut at the bound of an entry read - and
+ * must still be able to give up (#877).
+ */
+ @Test(timeOut = 600000)
+ public void testBulkStatementGivesUpAtItsOwnBound() throws Exception {
+ assertBoundedWhileRowsAreLocked("testBulkBound", JDBCStorage.StatementBound.BULK,
+ new BlockedOperation() {
+ @Override
+ public void run(JDBCStorage storage, TreeName tree) throws Exception {
+ // the importer is where "delete from <table>" - the bulk class - is reachable:
+ // AbstractTwoPhaseImportStrategy clears every tree before an import writes to it
+ try (final Importer importer = storage.startImport()) {
+ importer.clearTree(tree);
+ }
+ }
+ });
+ }
+
+ private interface BlockedOperation {
+ void run(JDBCStorage storage, TreeName tree) throws Exception;
+ }
+
+ /**
+ * Whether the failure the operation gave up with is the one its bound produced: the message of
+ * a statement classified as having reached its bound names the property that bounded it, and it
+ * arrives wrapped in whatever the storage throws to its caller.
+ */
+ private static boolean namesTheBound(Throwable failure, JDBCStorage.StatementBound bound) {
+ return namedInTheChain(failure, bound.property);
+ }
+
+ /**
+ * Whether the statement ran under the socket read timeout alone, which is what
+ * {@code timedOut()} says of one whose driver would not take the cancel. That degradation is by
+ * design - {@code JDBCStorage.setQueryTimeout()} warns once and carries on - and it is
+ * therefore silent: with a ceiling wide enough for the second layer, a run with the first one
+ * gone entirely ends at the backstop and passes as the bound doing its work.
+ */
+ private static boolean ranUnderTheBackstopAlone(Throwable failure) {
+ return namedInTheChain(failure, JDBCStorage.BACKSTOP_ALONE);
+ }
+
+ /** Cause hops walked below, as {@code JDBCStorage} bounds its own classifier: a guard against a cycle. */
+ private static final int MAX_CAUSE_HOPS = 16;
+
+ private static boolean namedInTheChain(Throwable failure, String text) {
+ // bounded by hops rather than by t != t.getCause(), which only catches a cause that is its
+ // own: a wrapper re-attaching an exception it has already wrapped makes a cycle of two, and
+ // walking that one spins until the harness times the whole suite out
+ Throwable t = failure;
+ for (int hops = 0; t != null && hops < MAX_CAUSE_HOPS; t = t.getCause(), hops++) {
+ if (t.getMessage() != null && t.getMessage().contains(text)) {
+ return true;
+ }
+ if (t == t.getCause()) {
+ break;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Runs the given operation while another session holds every row of the tree in an uncommitted
+ * transaction, with only the property of the given class bounding it: the operation must give
+ * up inside that bound instead of waiting for a lock that is never released.
+ */
+ private void assertBoundedWhileRowsAreLocked(String treeId, JDBCStorage.StatementBound bound, BlockedOperation blocked)
+ throws Exception {
+ final int boundSeconds = 5;
+ final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null);
+ final TreeName tree = new TreeName(treeId, "tree");
+ 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(1), value(1));
+ }
+ });
+ // another session takes an exclusive lock on every row of the table and keeps it: the
+ // same statement clearTree() issues, so it is known to parse on all four dialects
+ try (final Connection blocker = DriverManager.getConnection(getJdbcUrl())) {
+ blocker.setAutoCommit(false);
+ try (final Statement lock = blocker.createStatement()) {
+ lock.executeUpdate("delete from " + storage.getTableName(tree));
+ }
+ // the rows go back whatever the assertions below do with the run: the cleanup of
+ // this method drops the table, which is a bulk statement and unbounded here, so a
+ // lock still held would park it until the timeout of the harness and turn one
+ // failed assertion into a stalled build
+ try {
+ // only the class under test is bounded, so a pass through the other one cannot
+ // be mistaken for the bound working
+ for (final JDBCStorage.StatementBound each : JDBCStorage.StatementBound.values()) {
+ System.setProperty(each.property, each == bound ? Integer.toString(boundSeconds) : "0");
+ }
+ // the monotonic clock, which is what timedOut() measures the bound with: a step of
+ // the wall clock can neither lengthen nor shorten what the assertions below allow
+ final long startedAt = System.nanoTime();
+ Exception failure = null;
+ try {
+ blocked.run(storage, tree);
+ fail("the operation must give up while the rows it needs are locked");
+ } catch (Exception expected) {
+ failure = expected; // the bound was reached and the transaction rolled back
+ }
+ final long elapsed = (System.nanoTime() - startedAt) / 1000000L;
+ // The failure has to be the one the bound produces, not any failure at all: an
+ // operation that fell over at once for an unrelated reason would otherwise pass
+ // this test at t=0. timedOut() names the property in the message of everything it
+ // classifies as reaching the bound.
+ assertTrue(namesTheBound(failure, bound), "gave up with " + stackTraceToSingleLineString(failure)
+ + ", which does not name " + bound.property);
+ // And under the layer it is supposed to be under. The ceiling below has to be
+ // wide enough for the second one, since that is what ends the wait on oracle,
+ // and a ceiling that wide cannot tell a working first layer from a missing one:
+ // a driver that stops taking setQueryTimeout degrades to the backstop silently
+ // by design, ends there, and would be scored as the bound doing its work. The
+ // message says which layer it was, so this assertion can too.
+ assertFalse(ranUnderTheBackstopAlone(failure), "the driver would not take a query timeout, so "
+ + "the statement ran under the socket read timeout alone: "
+ + stackTraceToSingleLineString(failure));
+ // And it has to arrive at the bound rather than at something else that happens to
+ // end the wait inside a generous ceiling: with the bound deleted, mysql would still
+ // come back after its own innodb_lock_wait_timeout of 50 s, and the assertion has
+ // to fail then. The ceiling is what the bound really allows a statement, which is
+ // the second layer rather than the property: holdBackstop() arms the socket read
+ // timeout at the bound plus its margin on every engine, not only on oracle, and a
+ // run where the cancel of the driver does not land ends there. Scoring that as a
+ // failure would fail this suite for the second layer doing exactly what it exists
+ // to do - and on oracle, where a session in a row-lock enqueue never acts on the
+ // break its driver sends, that is not an edge case but the normal path.
+ final long ceilingSeconds = boundSeconds + JDBCStorage.BACKSTOP_MARGIN_SECONDS + 10;
+ // with a little slack under the bound: a driver keeps its timer in whole seconds and
+ // may report the cancel a few milliseconds before the bound is arithmetically due,
+ // which is the slack timedOut() classifies such a statement with
+ assertTrue(elapsed >= boundSeconds * 1000L - JDBCStorage.CLOCK_SLACK_MILLIS,
+ "gave up after " + elapsed + " ms, before its bound of "
+ + boundSeconds + " s: something other than the bound ended the wait");
+ assertTrue(elapsed < ceilingSeconds * 1000L, "gave up only after " + elapsed + " ms, past the "
+ + ceilingSeconds + " s this bound of " + boundSeconds + " s allows");
+ }finally {
+ // in a catch of its own: a rollback that throws would otherwise replace the
+ // assertion above, and the run would report an unrelated connection problem
+ // instead of the bound that was missed. Nothing is lost by swallowing it - a
+ // session that cannot roll back has no rows left locked either.
+ try {
+ blocker.rollback();
+ } catch (SQLException releasingTheRows) {
+ // the assertions above are the outcome of this test, not this
+ }
+ }
+ }
+ } finally {
+ for (final JDBCStorage.StatementBound each : JDBCStorage.StatementBound.values()) {
+ System.clearProperty(each.property);
+ }
+ try {
+ storage.write(new WriteOperation() {
+ @Override
+ public void run(WriteableTransaction txn) throws Exception {
+ txn.deleteTree(tree);
+ }
+ });
+ } catch (Exception ignored) {}
+ storage.close();
+ }
+ }
+
+ /**
* 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/BulkCursorTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/BulkCursorTest.java
new file mode 100644
index 0000000..9fb7915
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/BulkCursorTest.java
@@ -0,0 +1,365 @@
+/*
+ * 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.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.ByteArrayOutputStream;
+import java.lang.reflect.Field;
+
+import org.forgerock.opendj.ldap.ByteString;
+import org.forgerock.opendj.ldap.DN;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.backends.VerifyConfig;
+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.TreeName;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.opends.server.core.ServerContext;
+import org.opends.server.crypto.CryptoSuite;
+import org.opends.server.types.LDIFExportConfig;
+import org.testng.annotations.Test;
+
+/**
+ * Which cursor the walks of a whole tree ask for (#877).
+ * <p>
+ * {@code ReadableTransaction.openBulkCursor()} is a {@code default} answering exactly as
+ * {@code openCursor()} does, so every storage engine but the JDBC backend behaves the same either
+ * way and a call site turned back into {@code openCursor()} would compile, run, and stay invisible
+ * everywhere - while on the JDBC backend it would take the bound of a client operation, two
+ * minutes by default, and abort the walk of a tree larger than that. Some of these walks have
+ * nobody at a command line to see it happen: the load of the compressed schema and the read that
+ * checks id2entry is there, both on the path of {@code start-ds}, and the generation ID a
+ * replicated domain computes for itself the first time it starts, which is an export of the whole
+ * of id2entry.
+ * <p>
+ * Each test below pins one such call site twice: that the bulk cursor is what it asks for, and
+ * that it asks for no cursor of an operation at all. The second half only bites where the walk
+ * really runs its body, so a walk whose fixture holds a record - {@code iterateDN2ID} - is given
+ * one: with an empty tree the loop stops on its first step and every {@code never()} below it
+ * passes on a run that reached nothing.
+ * <p>
+ * Six call sites are pinned that way - {@code ExportJob}, the id2entry, dn2id and VLV walks of
+ * {@code VerifyJob}, and both trees of {@code PersistentCompressedSchema} - together with the
+ * children count each row of the dn2id walk reads and the total the progress report of a verify
+ * is sized with, the latter pinned one hop above its cursor. Three more are held by other means:
+ * {@code ID2Entry.afterOpen()} has {@code ID2EntryTest}, the override that gives the class its
+ * meaning has {@code JDBCStatementBoundTestCase}, and {@code VerifyJob.iterateID2ChildrenCount()}
+ * cannot be reverted at all, since {@code ID2ChildrenCount} exposes no cursor but the bulk one and
+ * the revert would not compile. The last three tests pin a delegation rather than its caller, which
+ * is all that is available for them.
+ * <p>
+ * What none of this covers is the single-row {@code ReadableTransaction.read()} these same walks
+ * make - {@code id2entry.get()} once per row of dn2id and of a VLV index - which has no bulk form
+ * in the SPI at all and takes the class of the transaction it is made through. That is a gap of
+ * the SPI rather than of a call site: a read by primary key is not the scan a cursor batch is, so
+ * what it risks is a lock wait rather than a walk cut short.
+ * <p>
+ * Three call sites are left uncovered and are named here rather than passed over: the attribute
+ * index of {@code verify-index} ({@code VerifyJob.iterateAttrIndex}), whose {@code MatchingRuleIndex}
+ * is {@code final} and so cannot be handed to this suite, and the two of {@code BackendStat}. All
+ * three walk a tree only on the command line of an operator.
+ */
+@SuppressWarnings("javadoc")
+@Test(groups = { "precommit", "pluggablebackend" }, sequential = true)
+public class BulkCursorTest extends DirectoryServerTestCase
+{
+ private final TreeName id2entryName = new TreeName("dc=example,dc=com", "id2entry");
+
+ @SuppressWarnings("unchecked")
+ private static Cursor<ByteString, ByteString> emptyCursor()
+ {
+ return mock(Cursor.class); // next() answers false, so the walk stops on its first step
+ }
+
+ /** A cursor over a single record, so that the body of a walk really runs once. */
+ @SuppressWarnings("unchecked")
+ private static Cursor<ByteString, ByteString> cursorOver(ByteString key, ByteString value)
+ {
+ final Cursor<ByteString, ByteString> cursor = mock(Cursor.class);
+ when(cursor.next()).thenReturn(true, false);
+ when(cursor.getKey()).thenReturn(key);
+ when(cursor.getValue()).thenReturn(value);
+ return cursor;
+ }
+
+ /** A transaction whose every cursor is the empty one above, whichever kind is asked for. */
+ private static ReadableTransaction transactionWithEmptyCursors()
+ {
+ final ReadableTransaction txn = mock(ReadableTransaction.class);
+ when(txn.openBulkCursor(any(TreeName.class))).thenReturn(emptyCursor());
+ when(txn.openCursor(any(TreeName.class))).thenReturn(emptyCursor());
+ return txn;
+ }
+
+ /**
+ * Gives a mock of a tree the name it would have been constructed with.
+ * {@code AbstractTree.getName()} is {@code public final} over a private field, so mockito cannot
+ * stub it and the instance it builds - whose constructor never runs - answers {@code null}. A
+ * production call of {@code openBulkCursor(index.getName())} then passes {@code null}, which
+ * {@code any(TreeName.class)} happily matches under the mockito pinned here: the assertion would
+ * accept a walk of any tree at all, the wrong one included.
+ */
+ private static <T extends AbstractTree> T named(T tree, TreeName name) throws Exception
+ {
+ final Field field = AbstractTree.class.getDeclaredField("name");
+ field.setAccessible(true);
+ field.set(tree, name);
+ return tree;
+ }
+
+ /**
+ * An {@code export-ldif} walks the whole of id2entry, and so does the generation ID a replicated
+ * domain computes for itself the first time it starts - {@code LDAPReplicationDomain
+ * .computeGenerationId()} exports the backend to compute it, with no client operation waiting on
+ * it and no operator watching it fail.
+ */
+ @Test
+ public void testAnExportWalksId2entryWithABulkCursor() throws Exception
+ {
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+ final EntryContainer entryContainer = mock(EntryContainer.class);
+ when(entryContainer.getID2Entry()).thenReturn(new ID2Entry(id2entryName, new DataConfig.Builder().build()));
+
+ // LDIFExportConfig is final and nothing is written here, the walk stopping on its first step
+ new ExportJob(new LDIFExportConfig(new ByteArrayOutputStream())).exportContainer(txn, entryContainer);
+
+ verify(txn).openBulkCursor(id2entryName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /** {@code verify-index} walks id2entry whole, checking every entry against the indexes. */
+ @Test
+ public void testAVerifyWalksId2entryWithABulkCursor() throws Exception
+ {
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+ final VerifyJob job = new VerifyJob(mock(RootContainer.class), mock(VerifyConfig.class));
+ job.id2entry = new ID2Entry(id2entryName, new DataConfig.Builder().build());
+
+ job.iterateID2Entry(txn);
+
+ verify(txn).openBulkCursor(id2entryName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * And it walks dn2id whole as well, rebuilding the children counts as it goes - which is a read
+ * of the children count tree per DN, inside that walk and belonging to it. Read as a client
+ * operation those would put the bound of an entry read over a verify nobody is waiting on, once
+ * for every DN of the backend, so the tree here holds a record: with an empty one the walk stops
+ * before its first row and the assertions below hold whatever the counters do.
+ */
+ @Test
+ public void testAVerifyWalksDn2idAndItsChildrenCountsWithBulkCursors() throws Exception
+ {
+ final TreeName dn2idName = new TreeName("dc=example,dc=com", "dn2id");
+ final TreeName id2childrenCountName = new TreeName("dc=example,dc=com", "id2childrencount");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+ // built before it is handed over: stubbing it inside the argument of thenReturn() would open a
+ // stubbing while the one it belongs to is still unfinished, which mockito refuses
+ final Cursor<ByteString, ByteString> dn2idRows =
+ cursorOver(ByteString.valueOfUtf8("dc=example,dc=com"), ByteString.valueOfLong(1));
+ when(txn.openBulkCursor(dn2idName)).thenReturn(dn2idRows);
+ final VerifyJob job = new VerifyJob(mock(RootContainer.class), mock(VerifyConfig.class));
+ job.dn2id = new DN2ID(dn2idName, DN.valueOf("dc=example,dc=com"));
+ // read for the one row above, and answered with nothing: the entry it points at is not what
+ // this suite is about, and a missing one is counted as an error rather than thrown
+ job.id2entry = new ID2Entry(id2entryName, new DataConfig.Builder().build());
+ job.id2childrenCount = new ID2ChildrenCount(id2childrenCountName);
+
+ job.iterateDN2ID(txn);
+
+ verify(txn).openBulkCursor(dn2idName);
+ verify(txn).openBulkCursor(id2childrenCountName); // the count of the DN the walk just passed
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /** A VLV index of a verify is walked whole too, key by key. */
+ @Test
+ public void testAVerifyWalksAVlvIndexWithABulkCursor() throws Exception
+ {
+ final TreeName vlvIndexName = new TreeName("dc=example,dc=com", "vlv.people");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+ final VerifyJob job = new VerifyJob(mock(RootContainer.class), mock(VerifyConfig.class));
+
+ job.iterateVLVIndex(txn, named(mock(VLVIndex.class), vlvIndexName), true);
+
+ verify(txn).openBulkCursor(vlvIndexName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * The compressed schema is loaded by walking both of its trees whole, while the backend opens:
+ * {@code RootContainer.open()} constructs it before a single client operation can run, and a
+ * walk cut short there is a backend that does not open at all.
+ */
+ @Test
+ public void testTheCompressedSchemaIsLoadedWithBulkCursors() throws Exception
+ {
+ final WriteableTransaction txn = mock(WriteableTransaction.class);
+ // both trees are there and empty: loadTrees() walks only a tree that exists (#873), and the
+ // record counts a mock answers with leave nothing to migrate from the legacy pair
+ when(txn.treeExists(any(TreeName.class))).thenReturn(true);
+ when(txn.openBulkCursor(any(TreeName.class))).thenReturn(emptyCursor());
+ when(txn.openCursor(any(TreeName.class))).thenReturn(emptyCursor());
+
+ new PersistentCompressedSchema(mock(ServerContext.class), "bulkCursorTest", mock(Storage.class), txn,
+ AccessMode.READ_ONLY);
+
+ verify(txn, times(2)).openBulkCursor(any(TreeName.class)); // the object classes and the attributes
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * An index walked whole - by {@code verify-index} or by {@code dbtest} - asks the transaction
+ * for a bulk cursor, while {@code Index.openCursor()} stays what an operation evaluating a
+ * filter takes. This pins the delegation rather than either of its call sites: the index a
+ * verify walks is a {@code MatchingRuleIndex}, which is {@code final} and cannot be handed to a
+ * mock, and the one {@code dbtest} walks is chosen inside {@code BackendStat}.
+ */
+ @Test
+ public void testAnIndexWalkedWholeAsksForABulkCursor() throws Exception
+ {
+ final TreeName indexName = new TreeName("dc=example,dc=com", "cn.caseIgnoreMatch");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+ final CryptoSuite cryptoSuite = mock(CryptoSuite.class);
+ when(cryptoSuite.isEncrypted()).thenReturn(false);
+ final DefaultIndex index =
+ new DefaultIndex(indexName, mock(State.class), 5, mock(EntryContainer.class), cryptoSuite);
+
+ index.openBulkCursor(txn);
+
+ verify(txn).openBulkCursor(indexName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * The children counts are walked whole by {@code verify-index} through {@code ShardedCounter}.
+ * The delegation again, its one call site - {@code VerifyJob.iterateID2ChildrenCount()} - being
+ * held by the compiler instead: {@code ID2ChildrenCount} exposes no cursor but this one, so a
+ * revert to {@code openCursor} does not compile.
+ */
+ @Test
+ public void testTheChildrenCountsAreWalkedWithABulkCursor() throws Exception
+ {
+ final TreeName id2childrenCountName = new TreeName("dc=example,dc=com", "id2childrencount");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ new ID2ChildrenCount(id2childrenCountName).openBulkCursor(txn);
+
+ verify(txn).openBulkCursor(id2childrenCountName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * The record check inside that walk is bulk as well: {@code VerifyJob.iterateID2ChildrenCount()}
+ * asks it once per record of the children count tree, so a cursor of a client operation there is
+ * the same hazard as one over the tree itself. The delegation again - that walk is private, and
+ * {@code containsEntryID} has no other caller to keep an operation-class form for.
+ */
+ @Test
+ public void testTheRecordCheckOfAWholeTreeWalkAsksForABulkCursor() throws Exception
+ {
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ new ID2Entry(id2entryName, new DataConfig.Builder().build()).containsEntryID(txn, new EntryID(1));
+
+ verify(txn).openBulkCursor(id2entryName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /**
+ * And a count read outside such a walk is a client operation, which the bulk read above must not
+ * quietly turn into: an LDAP search asking for {@code numSubordinates} reads one, and there a
+ * bound of a client operation is exactly what it should take.
+ */
+ @Test
+ public void testAChildrenCountOfAClientOperationStaysAnOperation() throws Exception
+ {
+ final TreeName id2childrenCountName = new TreeName("dc=example,dc=com", "id2childrencount");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ new ID2ChildrenCount(id2childrenCountName).getCount(txn, new EntryID(1));
+
+ verify(txn).openCursor(id2childrenCountName);
+ verify(txn, never()).openBulkCursor(any(TreeName.class));
+ }
+
+ /**
+ * The total is one such count read on one key, so it takes the class of whoever asks: the walk
+ * that reads it to size its progress report, or the client operation that reads the same total.
+ */
+ @Test
+ public void testTheTotalCountOfAWholeTreeWalkAsksForABulkCursor() throws Exception
+ {
+ final TreeName id2childrenCountName = new TreeName("dc=example,dc=com", "id2childrencount");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ new ID2ChildrenCount(id2childrenCountName).getTotalCount(txn, true);
+
+ verify(txn).openBulkCursor(id2childrenCountName);
+ verify(txn, never()).openCursor(any(TreeName.class));
+ }
+
+ /** And {@code cn=monitor} reading the same total is a client operation. */
+ @Test
+ public void testTheTotalCountOfAClientOperationStaysAnOperation() throws Exception
+ {
+ final TreeName id2childrenCountName = new TreeName("dc=example,dc=com", "id2childrencount");
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ new ID2ChildrenCount(id2childrenCountName).getTotalCount(txn);
+
+ verify(txn).openCursor(id2childrenCountName);
+ verify(txn, never()).openBulkCursor(any(TreeName.class));
+ }
+
+ /**
+ * The count a verify reads to size its progress report belongs to the walk it measures. Its
+ * three siblings - the record counts of dn2id, of the children count tree and of a VLV index -
+ * are bulk by the tree they count, and this one was the branch left reading as a client
+ * operation: it is also the only one a plain {@code verify-index} reaches, the other three
+ * being the {@code --clean} path.
+ * <p>
+ * Pinned on the container rather than on a cursor, that read being one hop further down:
+ * {@code getNumberOfEntriesInBaseDN0} to {@code ID2ChildrenCount.getTotalCount} to the cursor
+ * the two tests above pin.
+ */
+ @Test
+ public void testTheProgressCountOfAVerifyIsReadAsPartOfItsWalk() throws Exception
+ {
+ final DN baseDN = DN.valueOf("dc=example,dc=com");
+ final VerifyConfig verifyConfig = mock(VerifyConfig.class);
+ when(verifyConfig.getBaseDN()).thenReturn(baseDN);
+ final EntryContainer entryContainer = mock(EntryContainer.class);
+ final RootContainer rootContainer = mock(RootContainer.class);
+ when(rootContainer.getEntryContainer(baseDN)).thenReturn(entryContainer);
+ final ReadableTransaction txn = transactionWithEmptyCursors();
+
+ // false: the entry iterator, which is what a verify-index runs unless it was given --clean
+ new VerifyJob(rootContainer, verifyConfig).new ProgressTask(false, txn);
+
+ verify(entryContainer).getNumberOfEntriesInBaseDN0(txn, true);
+ verify(entryContainer, never()).getNumberOfEntriesInBaseDN0(txn);
+ }
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2EntryTest.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2EntryTest.java
new file mode 100644
index 0000000..b163315
--- /dev/null
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/pluggable/ID2EntryTest.java
@@ -0,0 +1,55 @@
+/*
+ * 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.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.forgerock.opendj.ldap.ByteString;
+import org.opends.server.DirectoryServerTestCase;
+import org.opends.server.backends.pluggable.spi.Cursor;
+import org.opends.server.backends.pluggable.spi.TreeName;
+import org.opends.server.backends.pluggable.spi.WriteableTransaction;
+import org.testng.annotations.Test;
+
+@SuppressWarnings("javadoc")
+@Test(groups = { "precommit", "pluggablebackend" }, sequential = true)
+public class ID2EntryTest extends DirectoryServerTestCase
+{
+ /**
+ * The read that checks the tree is there when a backend opens asks for a bulk cursor. Its first
+ * batch carries no key to seek on, so a storage engine sees a walk of the whole tree - on the
+ * JDBC backend against SQL Server, a scan and a sort of it, {@code k} being a
+ * {@code varbinary(max)} that cannot be an index key - and this runs once per base DN on every
+ * open, outside the try/catch of {@code BackendImpl.openBackend()}. Bounded as the work of a
+ * client operation, a large backend would stop opening at all (#877).
+ */
+ @Test
+ public void testTheReadThatOpensTheTreeAsksForABulkCursor() throws Exception
+ {
+ final TreeName name = new TreeName("dc=example,dc=com", "id2entry");
+ final WriteableTransaction txn = mock(WriteableTransaction.class);
+ @SuppressWarnings("unchecked")
+ final Cursor<ByteString, ByteString> cursor = mock(Cursor.class);
+ when(txn.openBulkCursor(name)).thenReturn(cursor);
+
+ new ID2Entry(name, new DataConfig.Builder().build()).open(txn, false);
+
+ verify(txn).openBulkCursor(name);
+ verify(cursor).next();
+ }
+}
--
Gitblit v1.10.0