diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java index f752ca518e..188f7ba2fb 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/CachedConnection.java @@ -95,12 +95,18 @@ static Connection getConnection(String connectionString, final int waitTime) thr return con; } } + Connection conNew = null; try { - final Connection conNew = DriverManager.getConnection(connectionString); + conNew = DriverManager.getConnection(connectionString); conNew.setAutoCommit(false); conNew.setTransactionIsolation(TRANSACTION_READ_COMMITTED); return new CachedConnection(connectionString, conNew); } catch (SQLException e) { // max_connection server error: try recursion for reuse connection + if (conNew != null) { // the connection was established but not set up: nothing else would close it + try { + conNew.close(); + } catch (SQLException e2) {} + } return getConnection(connectionString, (waitTime == 0) ? 1 : waitTime * 2); } } 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 09878862b3..f0e6557d60 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 @@ -34,11 +34,13 @@ import org.opends.server.types.RestoreConfig; import org.opends.server.util.BackupManager; +import java.io.Closeable; import java.nio.ByteBuffer; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.sql.*; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import static org.opends.server.backends.pluggable.spi.StorageUtils.addErrorMessage; import static org.opends.server.util.StaticUtils.stackTraceToSingleLineString; @@ -88,6 +90,14 @@ int execute(PreparedStatement statement) throws SQLException { return statement.executeUpdate(); } + // unlike execute(), tolerates statements that return a result set ("analyze table" on mysql) + void executeAny(PreparedStatement statement) throws SQLException { + if (logger.isTraceEnabled()) { + logger.trace(LocalizableMessage.raw("jdbc: %s",statement)); + } + statement.execute(); + } + Connection getConnection() throws Exception { return CachedConnection.getConnection(config.getDBDirectory()); } @@ -111,6 +121,10 @@ public StorageStatus getStorageStatus() { @Override public void close() { storageStatus = StorageStatus.lockedDown(LocalizableMessage.raw("closed")); + // a stamp that the database rejected is remembered for as long as the storage is open, so + // that it is not reissued for every tree on every open; disabling and re-enabling the + // backend is the way to try again once the privilege has been granted + unstampableTrees.clear(); } final LoadingCache tree2table = Caffeine.newBuilder() @@ -151,6 +165,567 @@ static String storedIdentifier(DatabaseMetaData metaData, String name) throws SQ return name; } + private static final String[] NO_ARGS=new String[0]; + + // Comment statements take a lock (a metadata lock on mysql, a schema modification lock on sql + // server, a ddl lock on oracle), and mysql and sql server wait for it without limit by + // default (lock_wait_timeout is a year, lock_timeout is infinite): a stamp could queue behind + // an unrelated transaction of another session on the same database and - on mysql - park + // every other query on the table behind itself. The stamp is a diagnostic aid, so every + // dialect is told to give up after this many seconds instead of waiting. + private static final int COMMENT_LOCK_TIMEOUT_SECONDS=5; + + // The comment statement runs on a connection of its own (newStampConnection() below), and a + // driver waits for a connect attempt without limit unless it is told otherwise: a database + // that keeps its established connections alive but accepts no new ones (a moved vip, a proxy + // at its connection limit) would otherwise hang the open of a tree - dsconfig + // create-backend-index opens one on a running server - instead of leaving a table unstamped. + // Every dialect gets the same bound, in the unit its own driver property takes. + private static final int STAMP_CONNECT_TIMEOUT_SECONDS=10; + + // Not one of the four drivers bounds the whole login attempt with its connect property alone: + // postgres and mysql apply theirs to socket.connect(), oracle's own reference says + // CONNECT_TIMEOUT "doesn't include user authentication", and the sql server driver leaves the + // read of the prelogin answer unbounded - TDSChannel.open() gives the socket + // min(what is left of loginTimeout, socketTimeout) and socketTimeout defaults to 0, which is + // "wait forever". Those reads - of the prelogin handshake, of tls, of authentication - are + // exactly where a proxy that accepts a connection and then goes quiet leaves the driver, so + // every dialect carries a read bound as well. All four are socket read timeouts, so the bound + // outlives the login phase and covers the comment statement too, which is why it is kept well + // clear of the lock bound above: the statement gives up on a contended lock long before the + // socket gives up on the server. On a mysql connection with tls (the sslMode=PREFERRED default + // of connector/j) the wall clock of a dead peer is twice this, since closing an SSLSocket + // drains input waiting for close_notify and pays the read bound a second time. + private static final int STAMP_READ_TIMEOUT_SECONDS=30; + + // Trees whose stamp failed for a reason that is not going to change by itself: an account that + // may not comment its tables (no ALTER privilege, for instance) would otherwise reissue the + // statement for every tree on every open. A failure that says nothing about the table - a lock + // timeout, a connection that broke - is not remembered (failureScope() below), so a contended + // moment does not leave the backend unstamped until it is restarted. Forgotten when the + // storage is closed, so re-enabling the backend is enough to try again once the privilege has + // been granted, without a restart of the server. + private final Set unstampableTrees=ConcurrentHashMap.newKeySet(); + + /** + * The engines whose comment statement, comment readback and statistics refresh this backend + * knows, with the session settings a comment statement needs: the driver properties that + * bound the connect attempt of the connection it runs on, and the statement that bounds its + * wait for the table lock. + */ + enum Dialect { + /** postgresql: lock_timeout takes milliseconds; connectTimeout bounds socket.connect(), loginTimeout the whole login the driver runs on a thread of its own, socketTimeout every read after it - all three in seconds. */ + POSTGRES("set lock_timeout = "+(COMMENT_LOCK_TIMEOUT_SECONDS*1000), + "connectTimeout", STAMP_CONNECT_TIMEOUT_SECONDS, "loginTimeout", STAMP_CONNECT_TIMEOUT_SECONDS, + "socketTimeout", STAMP_READ_TIMEOUT_SECONDS), + /** mysql: lock_wait_timeout takes seconds; connectTimeout bounds the socket connect and socketTimeout every read after it, both in milliseconds. */ + MYSQL("set session lock_wait_timeout="+COMMENT_LOCK_TIMEOUT_SECONDS, + "connectTimeout", STAMP_CONNECT_TIMEOUT_SECONDS*1000, "socketTimeout", STAMP_READ_TIMEOUT_SECONDS*1000), + /** oracle: ddl_lock_timeout takes seconds and defaults to 0 (give up at once), but it can be raised globally; the connect and read bounds take milliseconds. */ + ORACLE("alter session set ddl_lock_timeout="+COMMENT_LOCK_TIMEOUT_SECONDS, + "oracle.net.CONNECT_TIMEOUT", STAMP_CONNECT_TIMEOUT_SECONDS*1000, "oracle.jdbc.ReadTimeout", STAMP_READ_TIMEOUT_SECONDS*1000), + /** ms sql server: lock_timeout takes milliseconds; loginTimeout bounds the socket connect, in seconds, and socketTimeout the prelogin read it leaves open - and every read after it - in milliseconds. */ + MICROSOFT("set lock_timeout "+(COMMENT_LOCK_TIMEOUT_SECONDS*1000), + "loginTimeout", STAMP_CONNECT_TIMEOUT_SECONDS, "socketTimeout", STAMP_READ_TIMEOUT_SECONDS*1000); + + final String lockTimeoutSql; + // The driver properties bounding the login attempt of a stamp connection: the one bounding + // the socket connect and the one bounding the reads behind it, each in the unit its own + // driver takes. newStampConnection() hands the driver a copy of them: a driver is free to + // write into the map it is passed, and the sql server one gives a supplied property + // precedence over the same property of the url. + final Properties connectProperties=new Properties(); + + /** For the three drivers whose connect property and read property bound the login between them. */ + Dialect(String lockTimeoutSql, String connectProperty, int connectValue, String readProperty, int readValue) { + this.lockTimeoutSql=lockTimeoutSql; + connectProperties.setProperty(connectProperty, String.valueOf(connectValue)); + connectProperties.setProperty(readProperty, String.valueOf(readValue)); + } + + /** For the one driver bounding the login itself, on top of the socket connect and the reads behind it. */ + Dialect(String lockTimeoutSql, String connectProperty, int connectValue, String loginProperty, int loginValue, + String readProperty, int readValue) { + this(lockTimeoutSql, connectProperty, connectValue, readProperty, readValue); + connectProperties.setProperty(loginProperty, String.valueOf(loginValue)); + } + } + + static String driverNameOf(Connection con) { + return ((CachedConnection) con).parent.getClass().getName(); + } + + // The dialect behind a pooled connection, or null for an engine none of the statements of this + // class fit: it is left unstamped and its statistics untouched rather than fed untested SQL. + static Dialect dialectOf(Connection con) { + final String driverName=driverNameOf(con); + if (driverName.contains("postgres")) { + return Dialect.POSTGRES; + }else if (driverName.contains("mysql")) { + return Dialect.MYSQL; + }else if (driverName.contains("oracle")) { + return Dialect.ORACLE; + }else if (driverName.contains("microsoft")) { + return Dialect.MICROSOFT; + } + return null; + } + + /** Outcome of a comment stamp: openTree() ignores it, tests tell the cases apart. */ + enum CommentResult { + /** the table now carries its tree name */ + STAMPED, + /** the stored comment already matched: no statement was issued */ + UP_TO_DATE, + /** neither comment syntax nor readback is known for this engine */ + UNSUPPORTED, + /** the comment could not be read back or stored */ + FAILED + } + + // Splices a value into a single-quoted SQL literal for the comment DDL, which takes no bind + // parameters: doubles every quote, and every backslash on dialects where backslash is an + // escape character inside literals. The scan of the escaped result is defence in depth: it + // re-verifies that no quote (or live backslash) is left unpaired and able to terminate the + // literal, so a regression in the escaping throws instead of reaching the database. + private static String sqlLiteral(String value, boolean backslashIsEscape) { + final String escaped=(backslashIsEscape?value.replace("\\","\\\\"):value).replace("'","''"); + for (int i=0;i=escaped.length() || escaped.charAt(i+1)!=c) { + throw new IllegalArgumentException("unpaired "+c+" in SQL literal: "+escaped); + } + i++; + } + } + return "'"+escaped+"'"; + } + + // Whether backslash is an escape character inside string literals on this mysql connection: + // under the NO_BACKSLASH_ESCAPES sql mode it is an ordinary character, and doubling it there + // would store a comment that never matches its tree name - re-stamping the table forever. + // 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; + return sqlMode==null || !sqlMode.toUpperCase().contains("NO_BACKSLASH_ESCAPES"); + } + } + + // A connection of its own for the comment statements, outside the pool: they need session + // settings (the lock timeout above) that a pooled connection would carry over to whoever + // borrows it next, since CachedConnection.close() only rolls back. + Connection newStampConnection(Dialect dialect) throws SQLException { + final Properties properties=new Properties(); + properties.putAll(dialect.connectProperties); + final Connection con=DriverManager.getConnection(config.getDBDirectory(), properties); + try { + con.setAutoCommit(false); + executeSessionStatement(con, dialect.lockTimeoutSql); // give up instead of waiting for another session + // postgres undoes a plain SET when the transaction that ran it is rolled back, and a + // failed stamp is rolled back with the connection kept (StampSession.reset() below): + // commit the setting, or the first failure of a sweep would leave every tree after it + // stamped without the very bound this connection exists to carry. The session settings + // of the other three dialects are not transactional - the commit costs them an empty + // transaction. + con.commit(); + }catch (SQLException e) { // nothing else holds this connection yet: it would leak + try { + con.close(); + }catch (SQLException e2) {} + throw e; + } + return con; + } + + // The connection the comment statements of one sweep of openTree() calls share. Opening a + // backend opens every tree it holds (about 25 for a stock suffix), so a connection per stamp + // would mean that many physical connects on the first open after an upgrade - the one open + // that stamps them all. Opened lazily: a sweep that finds every comment up to date, which is + // every open after the first, opens nothing at all. + final class StampSession implements Closeable { + private Connection con; + + // Whether backslash escapes inside a literal on the connection above (mysql @@sql_mode). + // It is a session setting of a connection the whole sweep shares, so the sweep asks once + // instead of once per tree, and forgets it together with the session it describes. + private Boolean mysqlBackslashEscape; + + // Set when a stamp failed for a reason no other tree of this sweep would escape either: a + // connect that did not go through, a lock the statement gave up on. Each remaining tree + // would pay that same bound - or that same connect attempt - again, which is a backend + // open held for the bound times the number of its trees, all for a diagnostic aid. The + // sweep gives up instead; nothing about the trees is remembered, so the next open retries. + private boolean gaveUp; + + Connection connection(Dialect dialect) throws SQLException { + if (con==null) { + con=newStampConnection(dialect); + } + return con; + } + + // Asked by the mysql statement only, and only once the connection above is open. + boolean backslashIsEscape() throws SQLException { + if (mysqlBackslashEscape==null) { + mysqlBackslashEscape=isMysqlBackslashEscape(con); + } + return mysqlBackslashEscape; + } + + void giveUp() { + gaveUp=true; + } + + boolean hasGivenUp() { + return gaveUp; + } + + // A statement that failed can leave the session unusable (postgres refuses every further + // statement of the transaction with 25P02 until it is rolled back), so the stamp of the + // next tree gets a clean one: rolled back, or replaced when even the rollback fails. + void reset() { + if (con!=null) { + try { + con.rollback(); + }catch (SQLException e) { + close(); + } + } + } + + @Override + public void close() { + if (con!=null) { + try { + con.close(); + }catch (SQLException e) { + logger.trace(LocalizableMessage.raw("jdbc: unable to close the comment connection: %s", stackTraceToSingleLineString(e))); + } + con=null; + } + mysqlBackslashEscape=null; // it described the session that has just gone + } + } + + // 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. + private void executeSessionStatement(Connection con, String sql) throws SQLException { + try (final Statement statement=con.createStatement()) { + if (logger.isTraceEnabled()) { + logger.trace(LocalizableMessage.raw("jdbc: %s",sql)); + } + statement.execute(sql); + } + } + + // Table names are opaque SHA-224 hashes, so on the database side there is no way to tell + // which tree a table holds. Stamp each table with its tree name (visible in "\dt+" and the + // information schema) so database-level troubleshooting does not require recomputing hashes. + // Runs on a dedicated connection, never on the transaction that opened the tree: comment + // statements are DDL (an implicit commit on mysql and oracle), and a failing + // sp_addextendedproperty rolls the whole transaction back on sql server - either would + // corrupt work pending on the caller's connection (e.g. the trusted flag written by + // DefaultIndex.afterOpen()). The comment is a diagnostic aid: a failed attempt only logs and + // must not fail the backend. + CommentResult commentTable(TreeName treeName, Dialect dialect) { + try (final StampSession session=new StampSession()) { // a stamp of its own: no sweep to share a connection with + return commentTable(treeName, dialect, session); + } + } + + CommentResult commentTable(TreeName treeName, Dialect dialect, StampSession session) { + final String tableName=getTableName(treeName); + if (dialect==null) { // no comment syntax and readback known for other engines: leave the table unstamped + return CommentResult.UNSUPPORTED; + } + if (unstampableTrees.contains(treeName)) { // the database already rejected this one: do not ask again + return CommentResult.FAILED; + } + if (session.hasGivenUp()) { // an earlier tree of this sweep lost the session every tree of it needs + logger.debug(LocalizableMessage.raw("jdbc: table %s is left unstamped: the stamp of an earlier table of this open lost its connection", tableName)); + return CommentResult.FAILED; + } + final String treeComment=treeName.toString(); + try { + // The readback runs on the stamp connection, not on one borrowed from the pool: the + // caller of openTree() is inside a transaction and holding a pooled connection already, + // and a pool that cannot open a second one waits for a peer to return one - which here + // is the very thread that is waiting. The dialect comes from the caller's connection + // for the same reason: finding it out must not cost a borrow either. + final Connection con=session.connection(dialect); + // comment statements are DDL (metadata lock on mysql, ddl lock on oracle) and openTree() + // runs on every backend open: only stamp when the stored comment is absent or stale + final String storedComment=readStoredComment(con, dialect, tableName); + // end the read: this connection is shared by every tree of the sweep and must not hold + // a transaction open across all of them + con.commit(); + if (treeComment.equals(storedComment)) { + return CommentResult.UP_TO_DATE; + } + final String sql; + final String[] args; + switch (dialect) { + case MYSQL: // ALTER TABLE takes no binds; whether backslash escapes inside the literal depends on the sql mode of this session + sql="alter table "+tableName+" comment "+sqlLiteral(treeComment,session.backslashIsEscape()); + args=NO_ARGS; + break; + case MICROSOFT: // no COMMENT ON in t-sql: MS_Description extended property (procedure arguments take binds) + sql="declare @s sysname = schema_name()" + +" if exists (select 1 from sys.extended_properties where class=1 and major_id=object_id(?) and minor_id=0 and name='MS_Description')" + +" exec sys.sp_updateextendedproperty N'MS_Description', ?, N'SCHEMA', @s, N'TABLE', ?" + +" else" + +" exec sys.sp_addextendedproperty N'MS_Description', ?, N'SCHEMA', @s, N'TABLE', ?"; + args=new String[]{tableName, treeComment, tableName, treeComment, tableName}; + break; + case POSTGRES: // no binds in ddl; the E'' form keeps backslash an escape character regardless of standard_conforming_strings + sql="comment on table "+tableName+" is E"+sqlLiteral(treeComment,true); + args=NO_ARGS; + break; + case ORACLE: // no binds in ddl; backslash is never an escape character in oracle literals + sql="comment on table "+tableName+" is "+sqlLiteral(treeComment,false); + args=NO_ARGS; + break; + default: // a dialect this switch was never told about must not inherit another one's ddl + throw new IllegalStateException("no comment statement for dialect "+dialect); + } + try (final PreparedStatement statement=con.prepareStatement(sql)) { + for (int i=0;i pending=new ArrayDeque<>(); + final Set seen=Collections.newSetFromMap(new IdentityHashMap()); + if (failure!=null) { + pending.push(failure); + } + while (!pending.isEmpty()) { + final Throwable e=pending.pop(); + if (!seen.add(e)) { // a driver that chains an exception back to itself must not loop this walk + continue; + } + if (e.getCause()!=null) { + pending.push(e.getCause()); + } + if (!(e instanceof SQLException)) { + continue; + } + final SQLException sqlException=(SQLException) e; + if (sqlException.getNextException()!=null) { + pending.push(sqlException.getNextException()); + } + final FailureScope found=scopeOf(sqlException, dialect); + if (found==FailureScope.SESSION) { // nothing further down either chain can weaken this one + return FailureScope.SESSION; + } + if (found==FailureScope.MOMENT) { + scope=FailureScope.MOMENT; + } + } + return scope; + } + + // What one exception of the chain says on its own. + private static FailureScope scopeOf(SQLException e, Dialect dialect) { + final String sqlState=e.getSQLState(); + if (e instanceof SQLTransientConnectionException || e instanceof SQLNonTransientConnectionException + || e instanceof SQLRecoverableException // what oracle throws for a connection that has gone + || (sqlState!=null && sqlState.startsWith("08"))) { // connection exception + return FailureScope.SESSION; + } + if (e instanceof SQLTimeoutException || e instanceof SQLTransientException) { + return FailureScope.MOMENT; + } + if (dialect==null) { // the failure came before the engine was known + return FailureScope.TREE; + } + switch (dialect) { + case POSTGRES: // 55P03 lock not available: lock_timeout expired + return "55P03".equals(sqlState) ? FailureScope.MOMENT : FailureScope.TREE; + case MYSQL: // 1205 lock wait timeout exceeded + return e.getErrorCode()==1205 ? FailureScope.MOMENT : FailureScope.TREE; + case ORACLE: // ORA-00054 resource busy, ORA-04021 timeout occurred while waiting to lock object + return e.getErrorCode()==54 || e.getErrorCode()==4021 ? FailureScope.MOMENT : FailureScope.TREE; + case MICROSOFT: // 1222 lock request time out period exceeded + return e.getErrorCode()==1222 ? FailureScope.MOMENT : FailureScope.TREE; + default: // a dialect with no lock timeout code of its own here: its failures are not treated as ones of the moment + return FailureScope.TREE; + } + } + + // Returns the comment currently stored on the table, or null when there is none. The dialect is + // passed in rather than read off the connection: this runs on the stamp connection, which is + // not a pooled one, and only for the dialects commentTable() recognizes. + String readStoredComment(Connection con, Dialect dialect, String tableName) throws SQLException { + final String sql; + final String arg; + switch (dialect) { + case POSTGRES: + sql="select obj_description(to_regclass(?), 'pg_class')"; + arg=tableName; + break; + case MYSQL: + sql="select table_comment from information_schema.tables where table_schema=database() and table_name=?"; + arg=tableName; + break; + case ORACLE: + sql="select comments from user_tab_comments where table_name=?"; + arg=tableName.toUpperCase(); + break; + case MICROSOFT: + sql="select cast(value as nvarchar(4000)) from sys.extended_properties where class=1 and major_id=object_id(?) and minor_id=0 and name='MS_Description'"; + arg=tableName; + break; + default: // a dialect this switch was never told about must not inherit another one's catalog + throw new IllegalStateException("no table comment readback for dialect "+dialect); + } + try (final PreparedStatement statement=con.prepareStatement(sql)) { + statement.setString(1,arg); + try (final ResultSet rs=executeResultSet(statement)) { + return rs.next() ? rs.getString(1) : null; + } + } + } + + // Statistics upkeep after an import is bounded and can be turned off: gathering statistics of + // a freshly loaded table is a full scan on oracle (dbms_stats defaults to AUTO_SAMPLE_SIZE, + // and the entries themselves live in the blob column it reads), which a multi-million entry + // backend would otherwise pay in full, with no way to cap or skip it, after import-ldif has + // already reported its final status. + static final String STATISTICS_PROPERTY="org.openidentityplatform.opendj.jdbc.statistics"; + static final String STATISTICS_TIMEOUT_PROPERTY=STATISTICS_PROPERTY+".timeout"; + private static final int STATISTICS_TIMEOUT_SECONDS_DEFAULT=600; + + // A bulk load leaves the optimizer statistics of freshly created tables stale (a table that + // was never analyzed can make the planner badly misestimate the "where k>? order by k" cursor + // batches - see OpenIdentityPlatform/OpenDJ#859), so refresh them once the data is in place. + // Only the trees the import actually wrote are refreshed: rebuild-index imports a few index + // trees, and gathering statistics of the whole backend on its behalf is a full scan per + // table on oracle. Statistics upkeep is best-effort: a failure must not fail the import that + // produced the data, so failures are only logged - the return value makes them observable to tests. + boolean updateTableStatistics(Connection con, Collection trees) { + if (!Boolean.parseBoolean(System.getProperty(STATISTICS_PROPERTY,"true"))) { + logger.debug(LocalizableMessage.raw("jdbc: statistics refresh turned off by %s", STATISTICS_PROPERTY)); + return false; // nothing was refreshed + } + final Dialect dialect=dialectOf(con); + 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)); + boolean allRefreshed=true; + for (final TreeName treeName : trees) { + final String tableName=getTableName(treeName); + // The statement is chosen inside the try, so that the guard of the default branch + // degrades to "this table was not refreshed" like every other failure here: the + // contract above is that a refresh which failed never fails the import that produced + // the data, and a throw escaping this loop would break it. + try { + final String sql; + final String[] args; + switch (dialect) { + case POSTGRES: + sql="analyze "+tableName; + args=NO_ARGS; + break; + case MYSQL: + sql="analyze table "+tableName; + args=NO_ARGS; + break; + case ORACLE: + sql="begin dbms_stats.gather_table_stats(user, ?); end;"; + args=new String[]{tableName.toUpperCase()}; + break; + case MICROSOFT: + sql="update statistics "+tableName; + args=NO_ARGS; + break; + default: // a dialect this switch was never told about must not inherit another one's statement + throw new IllegalStateException("no statistics refresh for dialect "+dialect); + } + try (final PreparedStatement statement=con.prepareStatement(sql)) { + statement.setQueryTimeout(timeoutSeconds); // 0: wait without limit + for (int i=0;i T read(ReadOperation readOperation) throws Exception { @Override public void write(WriteOperation writeOperation) throws Exception { try (final Connection con=getConnection()) { + final WriteableTransactionTransactionImpl txn=new WriteableTransactionTransactionImpl(con); try { - writeOperation.run(new WriteableTransactionTransactionImpl(con)); + writeOperation.run(txn); con.commit(); } catch (Exception e) { try { con.rollback(); } catch (SQLException ex) {} throw e; + } finally { // the comment connection lives no longer than the trees it stamped + txn.stampSession.close(); } } } @@ -274,6 +857,11 @@ public long getRecordCount(TreeName treeName) { } private final class WriteableTransactionTransactionImpl extends ReadableTransactionImpl implements WriteableTransaction { + // Shared by every table this transaction stamps: opening a backend opens all its trees, + // and each stamp of its own connection would be a physical connect of its own. Closed by + // write() (and by ImporterImpl.close()) when the transaction is done with. + final StampSession stampSession=new StampSession(); + public WriteableTransactionTransactionImpl(Connection con) { super(con); if (!accessMode.isWriteable()) { @@ -363,6 +951,9 @@ public void openTree(TreeName treeName, boolean createOnDemand) { } } // mssql: k is varbinary(max), which cannot be an index key column - cursor batches stay unindexed there + // the dialect is taken off this transaction's own connection: finding it out must + // not cost a borrow from a pool this thread is already holding a connection of + commentTable(treeName, dialectOf(con), stampSession); } } @@ -397,6 +988,9 @@ public void deleteTree(TreeName treeName) { throw new StorageRuntimeException(e); } } + // forget the mapping so listTrees() consumers (updateTableStatistics) skip the dropped table + tree2table.invalidate(treeName); + unstampableTrees.remove(treeName); // a table recreated later deserves a fresh stamp attempt } @Override @@ -698,9 +1292,22 @@ private final class ImporterImpl implements Importer { final Connection con; final ReadableTransactionImpl txr; final WriteableTransactionTransactionImpl txw; + // The trees this import wrote: close() refreshes the statistics of these and only these, + // so rebuilding a single index does not gather statistics for the whole backend. A full + // import legitimately covers every tree - AbstractTwoPhaseImportStrategy.beforePhaseOne + // clears them all before the first record is written - including when the import is + // aborted, since close() runs from the try-with-resources of OnDiskMergeImporter. + final Set writtenTrees = ConcurrentHashMap.newKeySet(); + + // Set when the import failed or was cancelled. Its trees hold whatever the import got + // through before it stopped - beforePhaseOne cleared them all, so that can be nothing at + // all - and the operator is going to run it again, so there is nothing worth describing + // to the optimizer here: on oracle gathering those statistics is a full scan per table + // that would delay the report of a failure, or of a cancellation, by all of its duration. + volatile boolean aborted = false; final Boolean isOpen; - + public ImporterImpl() { isOpen=getStorageStatus().isWorking(); if (!isOpen) { @@ -719,27 +1326,47 @@ public ImporterImpl() { txw =new WriteableTransactionTransactionImpl(con); } + @Override + public void aborted() { + aborted = true; + } + @Override public void close() { try { - con.commit(); - con.close(); + try { + con.commit(); + if (aborted) { + logger.debug(LocalizableMessage.raw("jdbc: import aborted: statistics of the trees it wrote are left alone")); + }else { + updateTableStatistics(con, writtenTrees); + } + } finally { // the pooled connection must be returned even when the commit or a statistics statement throws + try { + con.close(); + } finally { + txw.stampSession.close(); + } + } } catch (SQLException e) { throw new StorageRuntimeException(e); - } - if (!isOpen) { - JDBCStorage.this.close(); + } finally { + if (!isOpen) { + JDBCStorage.this.close(); + } } } - + @Override public void clearTree(TreeName name) { txw.clearTree(name); + writtenTrees.add(name); } - + @Override public void put(TreeName treeName, ByteSequence key, ByteSequence value) { txw.put(treeName, key, value); + writtenTrees.add(treeName); } @Override diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java index 9adafb7c27..5b537910e8 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/OnDiskMergeImporter.java @@ -1194,6 +1194,33 @@ private OnDiskMergeImporter(String phase2ThreadNameTemplate, AbstractTwoPhaseImp } private void doImport(final Source source) throws InterruptedException, ExecutionException + { + try + { + importAllEntries(source); + } + catch (Throwable t) + { + // Cancellation lands here as well (see the InterruptedException below). The trees hold an + // incomplete import: the storage must not treat what is in them as the final data. Errors + // are caught too - an import killed by an OutOfMemoryError leaves the trees just as partial + // as one killed by an exception. + try + { + importStrategy.aborted(); + } + catch (Throwable notified) + { + // What went wrong here matters less than what brought the import down: the report of the + // failure is the point of this block. Concrete for the motivating case, an import killed + // by an OutOfMemoryError, where notifying the storage allocates. + t.addSuppressed(notified); + } + throw t; + } + } + + private void importAllEntries(final Source source) throws InterruptedException, ExecutionException { final long phaseOneStartTime = System.currentTimeMillis(); final PhaseOneWriteableTransaction transaction = new PhaseOneWriteableTransaction(importStrategy); @@ -1327,6 +1354,12 @@ void afterPhaseOne() closeSilently(bufferPool); } + /** Tells the storage that the import stopped before it was through, so that its data is not final. */ + void aborted() + { + importer.aborted(); + } + abstract Callable newPhaseTwoTask(TreeName treeName, Chunk source, PhaseTwoProgressReporter progressReporter); void afterPhaseTwo(EntryContainer entryContainer) 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 e01eb94b17..172388ef62 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 @@ -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; @@ -226,6 +227,14 @@ public ByteString read(TreeName name, ByteSequence key) return value; } + @Override + public void aborted() + { + traceEnter("aborted"); + importer.aborted(); + traceLeave("aborted"); + } + @Override public void close() { diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/Importer.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/Importer.java index 1012c443cc..3309faca28 100644 --- a/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/Importer.java +++ b/opendj-server-legacy/src/main/java/org/opends/server/backends/pluggable/spi/Importer.java @@ -12,6 +12,7 @@ * information: "Portions Copyright [year] [name of copyright owner]". * * Copyright 2014-2016 ForgeRock AS. + * Portions Copyright 2026 3A Systems, LLC. */ package org.opends.server.backends.pluggable.spi; @@ -74,6 +75,17 @@ public interface Importer extends Closeable */ SequentialCursor openCursor(TreeName treeName); + /** + * Notifies this importer that the import failed or was cancelled, before {@link #close()} runs: what the trees hold + * is an incomplete import that is going to be run again. Implementations doing work of their own in {@link #close()} + * on the assumption that the data is final - refreshing the optimizer statistics of a database, for instance - can + * skip it. The default implementation does nothing. + */ + default void aborted() + { + // nothing by default: an importer that treats a partial import like a complete one is not wrong, only wasteful + } + @Override void close(); } diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java new file mode 100644 index 0000000000..05b055ee02 --- /dev/null +++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/StampConnectionTestCase.java @@ -0,0 +1,352 @@ +/* + * 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.opendj.server.config.server.JDBCBackendCfg; +import org.opends.server.DirectoryServerTestCase; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.sql.Connection; +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.SQLNonTransientConnectionException; +import java.sql.SQLTimeoutException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotSame; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +/** + * The bounds of the connection the table comment statements run on, and the classification of a + * stamp that failed. None of it needs a database: the first two cases are about what the backend + * hands its driver and what the driver then does with a server that never answers, the last is a + * pure function of an exception. Keeping them out of the container suites is the point - those + * skip themselves whole when no docker is reachable, and a bound nothing exercises is a bound + * that can be deleted without a single test going red. + */ +@SuppressWarnings("javadoc") +public class StampConnectionTestCase extends DirectoryServerTestCase { + + /** + * A ceiling generous enough that a loaded machine cannot cross it, and far below what an + * unbounded driver does: a login left unbounded against a silent server does not come back at + * all - it sits in the read of the prelogin answer until something else tears the socket down. + */ + private static final long GIVE_UP_CEILING_SECONDS = 120; + + private ProbeDriver probeDriver; + + @BeforeClass + public void registerProbeDriver() throws SQLException { + probeDriver = new ProbeDriver(); + DriverManager.registerDriver(probeDriver); + } + + @AfterClass(alwaysRun = true) + public void deregisterProbeDriver() throws SQLException { + if (probeDriver != null) { + DriverManager.deregisterDriver(probeDriver); + } + } + + private static JDBCStorage storageFor(String url) { + final JDBCBackendCfg cfg = mockCfg(JDBCBackendCfg.class); + when(cfg.getBackendId()).thenReturn("stampProbe"); + when(cfg.getDBDirectory()).thenReturn(url); + return new JDBCStorage(cfg, null); + } + + /** + * Every dialect must bound both phases of a login attempt: the socket connect, and the reads + * behind it - of the prelogin handshake, of tls, of authentication. Not one of the four + * drivers covers both with a single property, the sql server one included: its loginTimeout + * bounds the connect and leaves the prelogin read open. + */ + @Test + public void testEveryDialectDeclaresBothBounds() { + for (final JDBCStorage.Dialect dialect : JDBCStorage.Dialect.values()) { + assertTrue(dialect.connectProperties.size() >= 2, + "the login of " + dialect + " is not bounded in both phases: " + dialect.connectProperties); + for (final String name : dialect.connectProperties.stringPropertyNames()) { + assertTrue(Integer.parseInt(dialect.connectProperties.getProperty(name)) > 0, + name + " of " + dialect + " bounds nothing: " + dialect.connectProperties.getProperty(name)); + } + } + } + + /** + * The declaration above is worth nothing unless it reaches the driver, and nothing else in the + * suite notices if it stops doing so: dropping the properties from the connect call leaves + * every stamp unbounded again, which no round-trip test can see against a database that + * answers. + */ + @Test + public void testStampConnectionHandsItsBoundsToTheDriver() throws Exception { + final JDBCStorage storage = storageFor(ProbeDriver.URL); + for (final JDBCStorage.Dialect dialect : JDBCStorage.Dialect.values()) { + probeDriver.lastProperties = null; + storage.newStampConnection(dialect).close(); + final Properties handed = probeDriver.lastProperties; + assertNotNull(handed, "no properties were handed to the driver for " + dialect); + for (final Map.Entry bound : dialect.connectProperties.entrySet()) { + assertEquals(handed.getProperty((String) bound.getKey()), bound.getValue(), + bound.getKey() + " of " + dialect + " did not reach the driver"); + } + // a driver is free to write into the map it is passed: the declaration must not be it + assertNotSame(handed, dialect.connectProperties, + "the driver was handed the declaration of " + dialect + " rather than a copy of it"); + } + } + + /** + * What the bounds are for: a database that keeps its established connections alive but accepts + * no new ones - a moved vip, a proxy at its connection limit - usually completes the tcp + * connect and then goes quiet, which leaves the driver in a read. Unbounded, that hangs the + * open of a tree; dsconfig create-backend-index opens one on a running server. + *

+ * The four dialects are attempted at once, so the suite pays the bound of the slowest of them + * rather than the sum of all four. + */ + @Test + public void testEveryDriverGivesUpOnASilentServer() throws Exception { + try (final SilentServer silent = new SilentServer()) { + final List> attempts = new ArrayList<>(); + for (final JDBCStorage.Dialect dialect : JDBCStorage.Dialect.values()) { + final JDBCStorage storage = storageFor(silent.urlFor(dialect)); + attempts.add(new Callable() { + @Override + public String call() { + final long startedAt = System.nanoTime(); + try (final Connection con = storage.newStampConnection(dialect)) { + return dialect + " connected to a server that never answered"; + } catch (Exception expected) { + // the failure itself is the point: which one it is belongs to the driver + } + final long tookSeconds = TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() - startedAt); + return tookSeconds <= GIVE_UP_CEILING_SECONDS ? null + : dialect + " took " + tookSeconds + "s to give up on a silent server"; + } + }); + } + final ExecutorService attempted = Executors.newFixedThreadPool(attempts.size()); + try { + final StringBuilder failures = new StringBuilder(); + for (final Future attempt : attempted.invokeAll(attempts, + GIVE_UP_CEILING_SECONDS * 2, TimeUnit.SECONDS)) { + final String failure = attempt.isCancelled() // the driver never came back at all + ? "a driver did not give up on a silent server within " + + (GIVE_UP_CEILING_SECONDS * 2) + "s" + : attempt.get(); + if (failure != null) { + failures.append(failure).append('\n'); + } + } + if (failures.length() > 0) { + fail(failures.toString()); + } + } finally { + attempted.shutdownNow(); + } + } + } + + /** + * A stamp that the database rejected is remembered, so that an account which may not comment + * its tables does not reissue the statement for every tree on every open. A stamp that lost + * its connection ends the sweep, since every tree behind it needs that same connection. A + * stamp that gave up on a lock is neither: the lock belongs to that one table. + */ + @Test + public void testFailureScopeTellsTheThreeApart() { + assertEquals(JDBCStorage.failureScope( + new SQLException("permission denied for table", "42501"), JDBCStorage.Dialect.POSTGRES), + JDBCStorage.FailureScope.TREE, "a rejected statement was not remembered"); + assertEquals(JDBCStorage.failureScope( + new SQLException("lock not available", "55P03"), JDBCStorage.Dialect.POSTGRES), + JDBCStorage.FailureScope.MOMENT, "a lock timeout was not read as one of the moment"); + assertEquals(JDBCStorage.failureScope( + new SQLException("connection closed", "08006"), JDBCStorage.Dialect.POSTGRES), + JDBCStorage.FailureScope.SESSION, "a connection exception did not end the sweep"); + assertEquals(JDBCStorage.failureScope( + new SQLNonTransientConnectionException("socket closed"), JDBCStorage.Dialect.MICROSOFT), + JDBCStorage.FailureScope.SESSION, "a connection exception of the driver did not end the sweep"); + assertEquals(JDBCStorage.failureScope( + new SQLTimeoutException("query timed out"), JDBCStorage.Dialect.MYSQL), + JDBCStorage.FailureScope.MOMENT, "a timeout was not read as a failure of the moment"); + } + + /** + * A driver reports the vendor error of a failed statement as the next exception of a generic + * one at least as often as it reports it as the cause. Reading only the cause chain classifies + * a lock timeout as a rejection, which leaves the tree unstamped until the next start over a + * moment of contention. + */ + @Test + public void testFailureScopeWalksBothChains() { + final SQLException reportedAsTheCause = new SQLException("statement failed", + new SQLException("lock wait timeout exceeded", "HY000", 1205)); + assertEquals(JDBCStorage.failureScope(reportedAsTheCause, JDBCStorage.Dialect.MYSQL), + JDBCStorage.FailureScope.MOMENT, "a lock timeout on the cause chain was missed"); + + final SQLException reportedAsTheNext = new SQLException("statement failed"); + reportedAsTheNext.setNextException(new SQLException("lock wait timeout exceeded", "HY000", 1205)); + assertEquals(JDBCStorage.failureScope(reportedAsTheNext, JDBCStorage.Dialect.MYSQL), + JDBCStorage.FailureScope.MOMENT, "a lock timeout on the next-exception chain was missed"); + + // a connection exception anywhere in either chain outweighs the rest: the session is gone + final SQLException connectionGone = new SQLException("statement failed"); + connectionGone.setNextException(new SQLException("communications link failure", "08S01")); + assertEquals(JDBCStorage.failureScope(connectionGone, JDBCStorage.Dialect.MYSQL), + JDBCStorage.FailureScope.SESSION, "a connection exception on the next-exception chain was missed"); + + // a driver that chains an exception back to itself must not make the walk loop + final SQLException selfReferring = new SQLException("statement failed"); + selfReferring.setNextException(selfReferring); + assertEquals(JDBCStorage.failureScope(selfReferring, JDBCStorage.Dialect.MYSQL), + JDBCStorage.FailureScope.TREE, "a self-referring chain was not walked to an end"); + } + + /** Accepts connections and answers nothing at all, the shape of a proxy at its connection limit. */ + private static final class SilentServer implements AutoCloseable { + private final ServerSocket listening; + private final List accepted = new ArrayList<>(); + private final Thread acceptor; + + SilentServer() throws IOException { + listening = new ServerSocket(0, 16, InetAddress.getLoopbackAddress()); + acceptor = new Thread(new Runnable() { + @Override + public void run() { + while (!Thread.currentThread().isInterrupted()) { + try { + final Socket socket = listening.accept(); + synchronized (accepted) { // held open, and never written to + accepted.add(socket); + } + } catch (IOException closed) { + return; + } + } + } + }, "silent-server"); + acceptor.setDaemon(true); + acceptor.start(); + } + + String urlFor(JDBCStorage.Dialect dialect) { + final String host = listening.getInetAddress().getHostAddress(); + final int port = listening.getLocalPort(); + switch (dialect) { + case POSTGRES: + return "jdbc:postgresql://" + host + ":" + port + "/probe?user=probe&password=probe"; + case MYSQL: + return "jdbc:mysql://" + host + ":" + port + "/probe?user=probe&password=probe"; + case ORACLE: + return "jdbc:oracle:thin:probe/probe@//" + host + ":" + port + "/probe"; + case MICROSOFT: + return "jdbc:sqlserver://" + host + ":" + port + ";databaseName=probe;user=probe;password=probe"; + default: + throw new IllegalStateException("no probe url for dialect " + dialect); + } + } + + @Override + public void close() throws IOException { + acceptor.interrupt(); + listening.close(); + synchronized (accepted) { + for (final Socket socket : accepted) { + try { + socket.close(); + } catch (IOException ignored) { + } + } + } + } + } + + /** Records the properties a stamp connection hands its driver, and connects to nothing. */ + private static final class ProbeDriver implements Driver { + static final String URL = "jdbc:stampprobe:"; + + volatile Properties lastProperties; + + @Override + public Connection connect(String url, Properties info) throws SQLException { + if (!acceptsURL(url)) { + return null; // not ours: DriverManager goes on to the next driver + } + lastProperties = info; + final Connection con = mock(Connection.class); + when(con.createStatement()).thenReturn(mock(Statement.class)); + return con; + } + + @Override + public boolean acceptsURL(String url) { + return url != null && url.startsWith(URL); + } + + @Override + public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) { + return new DriverPropertyInfo[0]; + } + + @Override + public int getMajorVersion() { + return 1; + } + + @Override + public int getMinorVersion() { + return 0; + } + + @Override + public boolean jdbcCompliant() { + return false; + } + + @Override + public java.util.logging.Logger getParentLogger() throws SQLFeatureNotSupportedException { + throw new SQLFeatureNotSupportedException(); + } + } +} 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 f7ad4ec14d..76760c1882 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 @@ -21,6 +21,7 @@ import org.opends.server.backends.pluggable.PluggableBackendImplTestCase; import org.opends.server.backends.pluggable.spi.AccessMode; import org.opends.server.backends.pluggable.spi.Cursor; +import org.opends.server.backends.pluggable.spi.Importer; import org.opends.server.backends.pluggable.spi.ReadOperation; import org.opends.server.backends.pluggable.spi.ReadableTransaction; import org.opends.server.backends.pluggable.spi.TreeName; @@ -35,17 +36,22 @@ import java.sql.Connection; import java.sql.DriverManager; +import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.NoSuchElementException; +import java.util.concurrent.atomic.AtomicInteger; import static org.forgerock.opendj.config.ConfigurationMock.mockCfg; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; @@ -320,6 +326,816 @@ public void run(WriteableTransaction txn) throws Exception { } } + /** + * Each table must be stamped with the tree name it stores: table names are opaque SHA-224 + * hashes, so without the comment there is no way to tell the trees apart on the database + * side (#859). The single quote in the base DN exercises the comment escaping. + */ + @Test + public void testTreeNameStoredAsTableComment() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + // a quote and a backslash in the tree name exercise the literal escaping (backslash is an escape character in mysql) + final TreeName tree = new TreeName("o=comment'te\\st", "dn2id"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + assertEquals(readTableComment(storage.getTableName(tree)), tree.toString()); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + String readTableComment(String tableName) throws Exception { + final String url = getJdbcUrl(); + final String sql; + if (url.startsWith("jdbc:postgresql")) { + sql = "select obj_description('" + tableName + "'::regclass, 'pg_class')"; + } else if (url.startsWith("jdbc:mysql")) { + sql = "select table_comment from information_schema.tables where table_schema=database() and table_name='" + tableName + "'"; + } else if (url.startsWith("jdbc:oracle")) { + sql = "select comments from user_tab_comments where table_name='" + tableName.toUpperCase() + "'"; + } else if (url.startsWith("jdbc:sqlserver")) { + // class=1 is the table itself: major_id is only unique within a class + sql = "select cast(value as nvarchar(4000)) from sys.extended_properties where class=1 and major_id=object_id('" + tableName + "') and minor_id=0 and name='MS_Description'"; + } else { + throw new SkipException("no table comment query for " + url); + } + try (final Connection con = DriverManager.getConnection(url); + final Statement st = con.createStatement(); + final ResultSet rs = st.executeQuery(sql)) { + return rs.next() ? rs.getString(1) : null; + } + } + + void writeTableComment(String tableName, String comment) throws Exception { + final String url = getJdbcUrl(); + final String sql; + if (url.startsWith("jdbc:postgresql") || url.startsWith("jdbc:oracle")) { + sql = "comment on table " + tableName + " is '" + comment + "'"; + } else if (url.startsWith("jdbc:mysql")) { + sql = "alter table " + tableName + " comment '" + comment + "'"; + } else if (url.startsWith("jdbc:sqlserver")) { + // exec arguments must be constants or variables: schema_name() cannot be passed inline + sql = "declare @s sysname = schema_name()" + + " exec sys.sp_updateextendedproperty N'MS_Description', N'" + comment + "', N'SCHEMA', @s, N'TABLE', N'" + tableName + "'"; + } else { + throw new SkipException("no table comment statement for " + url); + } + try (final Connection con = DriverManager.getConnection(url); + final Statement st = con.createStatement()) { + st.execute(sql); + } + } + + /** + * Removes the stored comment, so that the next stamp has to create one rather than replace + * it: on sql server that is sp_addextendedproperty, which is the statement reported to wait + * for an uncommitted row of another session. + */ + void clearTableComment(String tableName) throws Exception { + final String url = getJdbcUrl(); + if (!url.startsWith("jdbc:sqlserver")) { + writeTableComment(tableName, "stale"); // the other engines have one statement for both cases + return; + } + try (final Connection con = DriverManager.getConnection(url); + final Statement st = con.createStatement()) { + st.execute("declare @s sysname = schema_name()" + + " exec sys.sp_dropextendedproperty N'MS_Description', N'SCHEMA', @s, N'TABLE', N'" + tableName + "'"); + } + } + + /** The dialect of the database this suite runs against, as the backend detects it from the driver. */ + JDBCStorage.Dialect dialect() { + final String url = getJdbcUrl(); + if (url.startsWith("jdbc:postgresql")) { + return JDBCStorage.Dialect.POSTGRES; + } else if (url.startsWith("jdbc:mysql")) { + return JDBCStorage.Dialect.MYSQL; + } else if (url.startsWith("jdbc:oracle")) { + return JDBCStorage.Dialect.ORACLE; + } else if (url.startsWith("jdbc:sqlserver")) { + return JDBCStorage.Dialect.MICROSOFT; + } + throw new SkipException("no dialect for " + url); + } + + /** + * What this connection reports as its lock bound, in the unit and the rendering of its own + * engine, or null where reading it needs a privilege the test user does not have: oracle + * keeps ddl_lock_timeout in v$parameter, which an application user cannot select from. + */ + String sessionLockBound(Connection con) throws Exception { + final String url = getJdbcUrl(); + final String sql; + if (url.startsWith("jdbc:postgresql")) { + sql = "show lock_timeout"; + } else if (url.startsWith("jdbc:mysql")) { + sql = "select @@session.lock_wait_timeout"; + } else if (url.startsWith("jdbc:sqlserver")) { + sql = "select @@lock_timeout"; + } else { + return null; + } + try (final Statement st = con.createStatement(); + final ResultSet rs = st.executeQuery(sql)) { + return rs.next() ? rs.getString(1) : null; + } + } + + /** + * Comment statements are DDL (a metadata lock on mysql, a ddl lock on oracle), so a table + * whose stored comment already matches its tree name must not be re-stamped on subsequent + * opens - while a stale comment must be refreshed. + */ + @Test + public void testCommentStampSkippedWhenAlreadyStored() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName tree = new TreeName("o=commentSkip", "dn2id"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); // stamps the freshly created table + } + }); + assertEquals(readTableComment(storage.getTableName(tree)), tree.toString()); + // UP_TO_DATE and not FAILED: the statement was skipped, not rejected + assertEquals(storage.commentTable(tree, dialect()), JDBCStorage.CommentResult.UP_TO_DATE, "an up-to-date comment was re-stamped"); + writeTableComment(storage.getTableName(tree), "stale"); + assertEquals(storage.commentTable(tree, dialect()), JDBCStorage.CommentResult.STAMPED, "a stale comment was not re-stamped"); + assertEquals(readTableComment(storage.getTableName(tree)), tree.toString()); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * A stamp must never queue behind another session's transaction. Comment statements take a + * lock (a metadata lock on mysql, a schema modification lock on sql server) and both engines + * wait for it without limit by default - lock_wait_timeout is a year, lock_timeout is + * infinite - so an unbounded stamp could hang the backend open and, on mysql, park every + * other query on that table behind itself. Whether an uncommitted row of another session + * conflicts with the statement at all differs between engines and versions, so the assertion + * is on the timing: the call comes back rather than waiting for that transaction to end. + */ + @Test(timeOut = 180000) + public void testCommentStampGivesUpOnLock() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName tree = new TreeName("o=commentLock", "dn2id"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + final String tableName = storage.getTableName(tree); + clearTableComment(tableName); // no comment stored: the next attempt must issue a statement + try (final Connection blocker = DriverManager.getConnection(getJdbcUrl())) { + blocker.setAutoCommit(false); + try (final PreparedStatement st = blocker.prepareStatement("insert into " + tableName + " (h,k) values (?,?)")) { + st.setString(1, String.format("%1$-128s", "blocker").replace(' ', 'x')); + st.setBytes(2, new byte[]{1}); + st.executeUpdate(); + } + // the row is left uncommitted, so the lock it holds is still there + final long start = System.currentTimeMillis(); + final JDBCStorage.CommentResult result = storage.commentTable(tree, dialect()); + final long elapsedMs = System.currentTimeMillis() - start; + blocker.rollback(); + // giving up and stamping anyway are both fine here - the engines differ in whether an + // uncommitted row of another session conflicts with the comment statement at all. + // Waiting for that session to finish is what must never happen. + // the bound is 5 s (COMMENT_LOCK_TIMEOUT_SECONDS): the slack is for the connect and the + // statement around it, not for a regression of the bound itself + assertTrue(elapsedMs < 20000, "the comment statement waited " + elapsedMs + " ms for a lock, result " + result); + if (result == JDBCStorage.CommentResult.STAMPED) { // it reported success: the comment must be there + assertEquals(readTableComment(tableName), tree.toString()); + } + } + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * A failing comment stamp must never disturb the transaction that opened the tree: it used + * to roll back the caller's connection, silently discarding writes pending in the same + * transaction (the way DefaultIndex.afterOpen() writes the trusted flag between openTree() calls). + * The write pending during the failing stamp deliberately targets another tree: a statement + * left pending on the very table being stamped - a write, or on mysql any statement, since a + * transaction holds a shared metadata lock on every table it touched - would make the comment + * statement wait for the caller's own lock, which is a shape no production path has. + */ + @Test + public void testCommentFailureLeavesTransactionIntact() throws Exception { + final TreeName stamped = new TreeName("o=commentFailure", "dn2id"); + final TreeName written = new TreeName("o=commentFailure", "id2entry"); + final JDBCStorage setUp = new JDBCStorage(createBackendCfg(), null); + try { // create both tables up front, with a storage that stamps them normally + setUp.open(AccessMode.READ_WRITE); + setUp.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(stamped, true); + txn.openTree(written, true); + } + }); + } finally { + setUp.close(); + } + final AtomicInteger stampAttempts = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + Connection newStampConnection(Dialect dialect) throws SQLException { + stampAttempts.incrementAndGet(); + throw new SQLException("injected comment failure"); // no sql state, no vendor code: a rejection, not a failure of the moment + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.put(written, key(1), value(1)); // pending in this transaction... + txn.openTree(stamped, true); // ...while the comment machinery fails + } + }); + storage.read(new ReadOperation() { + @Override + public Void run(ReadableTransaction txn) throws Exception { + assertEquals(txn.read(written, key(1)), value(1), "failing comment stamp discarded a pending write"); + return null; + } + }); + // the failure is remembered: an unstampable table is not asked again while this backend is open + assertEquals(storage.commentTable(stamped, dialect()), JDBCStorage.CommentResult.FAILED); + assertEquals(stampAttempts.get(), 1, "a failed stamp was reissued"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(stamped); + txn.deleteTree(written); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * A stamp that failed for a reason of the moment - the lock timeout the statement is given, + * a connection that broke - must be attempted again: only a failure saying that this table + * cannot be commented at all is remembered, or one contended moment would leave a backend + * unstamped until it is restarted. + */ + @Test + public void testTransientStampFailureIsRetried() throws Exception { + final TreeName tree = new TreeName("o=transientStamp", "dn2id"); + final AtomicInteger stampAttempts = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + Connection newStampConnection(Dialect dialect) throws SQLException { + stampAttempts.incrementAndGet(); + throw new SQLException("injected connection failure", "08006"); // connection exception: a failure of the moment + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); // stamp #1, fails + } + }); + assertEquals(storage.commentTable(tree, dialect()), JDBCStorage.CommentResult.FAILED); + assertEquals(stampAttempts.get(), 2, "a stamp that failed for a reason of the moment was not attempted again"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * Opening a backend opens every tree it holds - about 25 for a stock suffix - and the first + * open after an upgrade stamps them all: the trees of one open must share one connection + * rather than make a physical connect each. One per open is what the comment machinery costs, + * readback included - the readback runs on that same connection, because the thread doing the + * open is inside a transaction and holding a pooled connection already. + */ + @Test + public void testCommentStampsShareOneConnection() throws Exception { + final TreeName[] trees = { + new TreeName("o=commentSweep", "dn2id"), + new TreeName("o=commentSweep", "id2entry"), + new TreeName("o=commentSweep", "state") }; + final AtomicInteger connects = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + Connection newStampConnection(Dialect dialect) throws SQLException { + connects.incrementAndGet(); + return super.newStampConnection(dialect); + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); // freshly created: every one of them is stamped + } + } + }); + for (final TreeName tree : trees) { + assertEquals(readTableComment(storage.getTableName(tree)), tree.toString()); + } + assertEquals(connects.get(), 1, "the stamps of one open did not share a connection"); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); + } + } + }); + // three trees, one more connect: the open that finds every comment in place issues no + // statement and takes no lock, and pays one connection for the whole sweep either way + assertEquals(connects.get(), 2, "the trees of an open that found every comment in place did not share a connection"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.deleteTree(tree); + } + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * The bound a stamp connection is given must survive a stamp that failed. Postgres undoes a + * plain SET when the transaction that ran it is rolled back, and a failed stamp is rolled + * back with the connection kept and reused - one connection serves every tree of a backend + * open - so every tree stamped after the first failure used to run with no bound at all, + * which is what the bound exists to prevent. + */ + @Test + public void testLockBoundSurvivesAFailedStamp() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + // no table was ever created for this tree, so its comment statement fails - on a connection + // that stays usable, which is the case the session rolls back rather than replaces + final TreeName missing = new TreeName("o=lockBound", "neverCreated"); + try { + storage.open(AccessMode.READ_WRITE); + final JDBCStorage.Dialect dialect = dialect(); + final String bound; + try (final Connection fresh = storage.newStampConnection(dialect)) { + bound = sessionLockBound(fresh); // what a connection carrying the bound reports + } + try (final JDBCStorage.StampSession session = storage.new StampSession()) { + assertEquals(storage.commentTable(missing, dialect, session), JDBCStorage.CommentResult.FAILED, + "stamping a table that does not exist was reported as done"); + if (bound != null) { // oracle: ddl_lock_timeout is only in v$parameter, which the test user cannot read + assertEquals(sessionLockBound(session.connection(dialect)), bound, + "the lock bound was lost when the failed stamp was rolled back"); + } + } + } finally { + storage.close(); + } + } + + /** + * A stamp that lost its connection ends the sweep it happened in: every tree behind it needs + * that same connection, so each would pay the same connect attempt again. That is about 25 of + * them for a stock suffix, all for a diagnostic aid. Nothing is remembered, so the next open + * tries again. + */ + @Test + public void testConnectionFailureEndsTheSweep() throws Exception { + final TreeName[] trees = { + new TreeName("o=sweepGiveUp", "dn2id"), + new TreeName("o=sweepGiveUp", "id2entry"), + new TreeName("o=sweepGiveUp", "state") }; + final AtomicInteger stampAttempts = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + Connection newStampConnection(Dialect dialect) throws SQLException { + stampAttempts.incrementAndGet(); + throw new SQLException("injected connection failure", "08006"); // connection exception: the session is gone + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); + } + } + }); + assertEquals(stampAttempts.get(), 1, "a connection that was gone was paid once per tree of the same open"); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); + } + } + }); + assertEquals(stampAttempts.get(), 2, "the open after a lost connection did not try again"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.deleteTree(tree); + } + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * A lock belongs to the table it is held on, so a stamp that gave up on one must not cost the + * trees behind it their comments: the trees of an open are stamped in a fixed order, and a + * table left permanently contended by another session would otherwise mean nothing is ever + * stamped, on any open. Nothing is remembered either - the open that follows stamps the table + * whose moment has passed. + *

+ * The failure is injected at the readback rather than at the comment statement, which is built + * inline; what is under test is the classification of the failure and what the sweep does with + * it, and those do not depend on which of the two statements produced it. + */ + @Test + public void testContendedTableDoesNotEndTheSweep() throws Exception { + final TreeName[] trees = { + new TreeName("o=sweepContended", "dn2id"), + new TreeName("o=sweepContended", "id2entry"), + new TreeName("o=sweepContended", "state") }; + final AtomicInteger contended = new AtomicInteger(1); // the first tree, for one sweep only + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + String readStoredComment(Connection con, Dialect dialect, String tableName) throws SQLException { + if (tableName.equals(getTableName(trees[0])) && contended.getAndDecrement() > 0) { + throw lockTimeoutOf(dialect); // as if another session held this one table + } + return super.readStoredComment(con, dialect, tableName); + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); + } + } + }); + assertNotEquals(readTableComment(storage.getTableName(trees[0])), trees[0].toString(), + "the contended table was stamped anyway"); + for (int i = 1; i < trees.length; i++) { + assertEquals(readTableComment(storage.getTableName(trees[i])), trees[i].toString(), + "one contended table cost the trees behind it their comments"); + } + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); + } + } + }); + assertEquals(readTableComment(storage.getTableName(trees[0])), trees[0].toString(), + "a table left unstamped by a contended moment was not stamped by the open that followed"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.deleteTree(tree); + } + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** The failure a dialect reports when a statement gave up on the lock bound it was given. */ + static SQLException lockTimeoutOf(JDBCStorage.Dialect dialect) { + switch (dialect) { + case POSTGRES: + return new SQLException("canceling statement due to lock timeout", "55P03"); + case MYSQL: + return new SQLException("Lock wait timeout exceeded; try restarting transaction", "HY000", 1205); + case ORACLE: + return new SQLException("ORA-00054: resource busy and acquire with NOWAIT specified", "61000", 54); + case MICROSOFT: + return new SQLException("Lock request time out period exceeded", "HY000", 1222); + default: + throw new IllegalStateException("no lock timeout failure for dialect " + dialect); + } + } + + /** + * @@sql_mode decides whether a backslash escapes inside the comment literal. It belongs to + * the session, and the stamps of one open share a connection, so it is asked once for the + * whole sweep rather than once per tree - and only on mysql, the one engine whose literal + * depends on it. + */ + @Test + public void testSqlModeProbedOncePerSweep() throws Exception { + final TreeName[] trees = { + new TreeName("o=sqlModeProbe", "dn2id"), + new TreeName("o=sqlModeProbe", "id2entry"), + new TreeName("o=sqlModeProbe", "state") }; + final AtomicInteger probes = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + boolean isMysqlBackslashEscape(Connection con) throws SQLException { + probes.incrementAndGet(); + return super.isMysqlBackslashEscape(con); + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.openTree(tree, true); // freshly created: every one of them is stamped + } + } + }); + for (final TreeName tree : trees) { + assertEquals(readTableComment(storage.getTableName(tree)), tree.toString()); + } + assertEquals(probes.get(), dialect() == JDBCStorage.Dialect.MYSQL ? 1 : 0, + "the sql mode of one sweep was not asked exactly once"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + for (final TreeName tree : trees) { + txn.deleteTree(tree); + } + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + // The bounds of a stamp connection are covered by StampConnectionTestCase: what they are worth + // is whether they reach the driver and whether the driver then gives up on a server that never + // answers, and neither needs - nor can be staged by - a database container. + + /** + * An import that failed or was cancelled leaves trees holding an incomplete import that is + * going to be run again: refreshing statistics of it describes data nobody will query, and on + * oracle it is a full scan per table between the failure and its report. + */ + @Test + public void testAbortedImportSkipsStatistics() throws Exception { + final TreeName tree = new TreeName("o=abortedImport", "dn2id"); + final AtomicInteger refreshes = new AtomicInteger(); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + boolean updateTableStatistics(Connection con, Collection trees) { + refreshes.incrementAndGet(); + return super.updateTableStatistics(con, trees); + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + try (final Importer importer = storage.startImport()) { + importer.put(tree, key(1), value(1)); + importer.aborted(); // what OnDiskMergeImporter reports when the import throws or is cancelled + } + assertEquals(refreshes.get(), 0, "statistics were refreshed for an import that was aborted"); + try (final Importer importer = storage.startImport()) { + importer.put(tree, key(2), value(2)); + } + assertEquals(refreshes.get(), 1, "statistics were not refreshed for an import that finished"); + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * The statistics refresh must be possible to turn off: on oracle it gathers with + * AUTO_SAMPLE_SIZE, a full scan of every table the import wrote. + */ + @Test + public void testStatisticsRefreshCanBeTurnedOff() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName tree = new TreeName("o=statisticsOff", "dn2id"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + System.setProperty(JDBCStorage.STATISTICS_PROPERTY, "false"); + try (final Connection con = CachedConnection.getConnection(getJdbcUrl())) { + assertFalse(storage.updateTableStatistics(con, Collections.singleton(tree)), + "the refresh ran with " + JDBCStorage.STATISTICS_PROPERTY + "=false"); + } + } finally { + System.clearProperty(JDBCStorage.STATISTICS_PROPERTY); + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** deleteTree() must forget the tree: statistics refresh iterates known trees and must skip dropped tables. */ + @Test + public void testDeleteTreeForgetsTree() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName tree = new TreeName("o=deleteTree", "dn2id"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + assertTrue(storage.listTrees().contains(tree)); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + assertFalse(storage.listTrees().contains(tree), "deleteTree() left the tree in the tree-to-table cache"); + } finally { + storage.close(); + } + } + + /** A bulk import must refresh optimizer statistics: fresh tables were never analyzed (#859). */ + @Test + public void testImportRefreshesTableStatistics() throws Exception { + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null); + final TreeName tree = new TreeName("testImportAnalyze", "tree"); + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); + } + }); + suspendAutomaticStatistics(storage.getTableName(tree)); + try (final Importer importer = storage.startImport()) { + for (int i = 0; i < 40; i++) { + importer.put(tree, key(i), value(i)); + } + } + assertTableStatisticsFresh(storage.getTableName(tree)); + // import swallows statistics failures by design: assert directly that the + // dialect-specific refresh statement is accepted by this database + try (final Connection con = CachedConnection.getConnection(getJdbcUrl())) { + assertTrue(storage.updateTableStatistics(con, Collections.singleton(tree)), "statistics refresh reported failures"); + } + } finally { + try { + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.deleteTree(tree); + } + }); + } catch (Exception ignored) {} + storage.close(); + } + } + + /** + * Suspends the automatic statistics upkeep of the engines that have it, so that what the + * assertion below sees was produced by the refresh of the import and by nothing else: InnoDB + * recalculates innodb_table_stats.n_rows on its own (innodb_stats_auto_recalc is on by + * default), which would let the assertion pass with no "analyze table" ever issued. + */ + void suspendAutomaticStatistics(String tableName) throws Exception { + final String url = getJdbcUrl(); + if (!url.startsWith("jdbc:mysql")) { + return; // nothing refreshes what is asserted below on the other engines within a test run + } + try (final Connection con = DriverManager.getConnection(url); + final Statement st = con.createStatement()) { + st.execute("alter table " + tableName + " stats_auto_recalc=0"); + } + } + + void assertTableStatisticsFresh(String tableName) throws Exception { + final String url = getJdbcUrl(); + final String sql; + if (url.startsWith("jdbc:postgresql")) { + // reltuples stays -1/0 until the first ANALYZE + sql = "select reltuples::bigint from pg_class where relname='" + tableName + "'"; + } else if (url.startsWith("jdbc:oracle")) { + // num_rows stays null until dbms_stats gathers statistics + sql = "select num_rows from user_tables where table_name='" + tableName.toUpperCase() + "'"; + } else if (url.startsWith("jdbc:mysql")) { + // n_rows in the persistent stats table is refreshed by ANALYZE TABLE, and - with the + // automatic recalculation suspended above - by nothing else: it stays 0 without it + sql = "select n_rows from mysql.innodb_table_stats where database_name=database() and table_name='" + tableName + "'"; + } else if (url.startsWith("jdbc:sqlserver")) { + // last_updated stays null until the first UPDATE STATISTICS + sql = "select count(*) from sys.stats s cross apply sys.dm_db_stats_properties(s.object_id, s.stats_id) p" + + " where s.object_id=object_id('" + tableName + "') and p.last_updated is not null"; + } else { + throw new SkipException("no statistics query for " + url); + } + try (final Connection con = DriverManager.getConnection(url); + final Statement st = con.createStatement(); + final ResultSet rs = st.executeQuery(sql)) { + assertTrue(rs.next(), "table " + tableName + " not found"); + final long rows = rs.getLong(1); + assertFalse(rs.wasNull(), "statistics were never gathered for " + tableName); + assertTrue(rows > 0, "statistics of " + tableName + " look stale: " + rows); + } + } + /** Cursor operations must keep working when the tree spans several "fetchsize" batches. */ @Test public void testCursorCrossesFetchSizeBatches() throws Exception {