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 fcf3737df4..95a9112e6b 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 @@ -39,6 +39,7 @@ 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 +89,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()); } @@ -134,6 +143,182 @@ String getTableName(TreeName treeName) { return tree2table.get(treeName); } + private static final String[] NO_ARGS=new String[0]; + + static String driverNameOf(Connection con) { + return ((CachedConnection) con).parent.getClass().getName(); + } + + // 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+"'"; + } + + // 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 pooled 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()). Returns true when the comment was stamped; the comment is a + // diagnostic aid, so a failed attempt only returns false and must not fail the backend. + boolean commentTable(TreeName treeName) { + final String tableName=getTableName(treeName); + try (final Connection con=getConnection()) { + final String driverName=driverNameOf(con); + final boolean postgres=driverName.contains("postgres"); + final boolean mysql=driverName.contains("mysql"); + final boolean oracle=driverName.contains("oracle"); + final boolean microsoft=driverName.contains("microsoft"); + if (!postgres && !mysql && !oracle && !microsoft) { // no comment syntax and readback known for other engines: leave the table unstamped + return false; + } + final String treeComment=treeName.toString(); + // 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 + if (treeComment.equals(readStoredComment(con, tableName))) { + return false; + } + final String sql; + final String[] args; + if (mysql) { // ALTER TABLE takes no binds; backslash is an escape character in mysql literals + sql="alter table "+tableName+" comment "+sqlLiteral(treeComment,true); + args=NO_ARGS; + }else if (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}; + }else if (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; + }else { // 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; + } + try (final PreparedStatement statement=con.prepareStatement(sql)) { + for (int i=0;i? 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) { + final String driverName=driverNameOf(con); + final boolean postgres=driverName.contains("postgres"); + final boolean mysql=driverName.contains("mysql"); + final boolean oracle=driverName.contains("oracle"); + final boolean microsoft=driverName.contains("microsoft"); + if (!postgres && !mysql && !oracle && !microsoft) { // no portable statistics refresh for other engines + return true; + } + boolean allRefreshed=true; + for (final TreeName treeName : trees) { + final String tableName=getTableName(treeName); + final String sql; + final String[] args; + if (postgres) { + sql="analyze "+tableName; + args=NO_ARGS; + }else if (mysql) { + sql="analyze table "+tableName; + args=NO_ARGS; + }else if (oracle) { + sql="begin dbms_stats.gather_table_stats(user, ?); end;"; + args=new String[]{tableName.toUpperCase()}; + }else { + sql="update statistics "+tableName; + args=NO_ARGS; + } + try (final PreparedStatement statement=con.prepareStatement(sql)) { + for (int i=0;i writtenTrees = ConcurrentHashMap.newKeySet(); final Boolean isOpen; - + public ImporterImpl() { isOpen=getStorageStatus().isWorking(); if (!isOpen) { @@ -694,24 +888,31 @@ public ImporterImpl() { @Override public void close() { try { - con.commit(); - con.close(); + try { + con.commit(); + updateTableStatistics(con, writtenTrees); + } finally { // the pooled connection must be returned even when the commit or a statistics statement throws + con.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/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java index 472f62fc9a..65a1ed0af5 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; @@ -39,6 +40,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.NoSuchElementException; @@ -269,6 +271,255 @@ 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")) { + sql = "select cast(value as nvarchar(4000)) from sys.extended_properties where 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); + } + } + + /** + * 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()); + assertFalse(storage.commentTable(tree), "an up-to-date comment was re-stamped"); + writeTableComment(storage.getTableName(tree), "stale"); + assertTrue(storage.commentTable(tree), "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 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). + */ + @Test + public void testCommentFailureLeavesTransactionIntact() throws Exception { + final TreeName tree = new TreeName("o=commentFailure", "dn2id"); + final JDBCStorage storage = new JDBCStorage(createBackendCfg(), null) { + @Override + String readStoredComment(Connection con, String tableName) throws SQLException { + throw new SQLException("injected comment readback failure"); + } + }; + try { + storage.open(AccessMode.READ_WRITE); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.openTree(tree, true); // create the table up front + } + }); + storage.write(new WriteOperation() { + @Override + public void run(WriteableTransaction txn) throws Exception { + txn.put(tree, key(1), value(1)); // pending in this transaction... + txn.openTree(tree, true); // ...while the comment machinery fails + } + }); + storage.read(new ReadOperation() { + @Override + public Void run(ReadableTransaction txn) throws Exception { + assertEquals(txn.read(tree, key(1)), value(1), "failing comment stamp discarded a pending write"); + return null; + } + }); + } finally { + 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); + } + }); + 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(); + } + } + + 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 + 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 {