From 7cebc65e3c86e7b49c3bc6f782745c182f59acf3 Mon Sep 17 00:00:00 2001
From: Valery Kharseko <vharseko@3a-systems.ru>
Date: Mon, 21 Sep 2026 10:07:24 +0000
Subject: [PATCH] [#1012] Pin the commit of the post-import statistics refresh, and say why it is there (#1016)
---
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java | 13 +++++-
opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java | 41 ++++++++++++++++++++
opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java | 9 ++++
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java b/opendj-server-legacy/src/main/java/org/opends/server/backends/jdbc/JDBCStorage.java
index 6721873..4f56aa4 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
@@ -2842,6 +2842,15 @@
}
return null;
});
+ // Committed here, per tree, rather than left to the caller: the connection of an import
+ // goes back to the pool as soon as this refresh is over, and CachedConnection.close()
+ // rolls back before the pool hands it on - so a refresh left inside the transaction of
+ // the borrow would be discarded by its own return, on the one engine where the database
+ // does not commit it for us. On postgres the rows ANALYZE writes to pg_statistic are
+ // ordinary catalog rows, and a rollback takes with them the histogram the "where k>?
+ // order by k" batches of #859 need, leaving only the pg_class.reltuples it writes in
+ // place; mysql (implicit commit), oracle (dbms_stats commits) and sql server commit the
+ // statement themselves, which is why no container suite pins this (issue #1012).
con.commit();
}
}catch (Exception e) {
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java
index 8fe246e..78d5fac 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/JDBCStatementBoundTestCase.java
@@ -46,6 +46,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static org.forgerock.opendj.config.ConfigurationMock.mockCfg;
import static org.mockito.Mockito.any;
@@ -1224,6 +1225,43 @@
}
/**
+ * Every statistics statement is committed where it runs, one commit per tree. The connection of
+ * an import goes back to the pool as soon as the refresh is over and
+ * {@code CachedConnection.close()} rolls back before the pool hands it on, so a refresh left
+ * inside the transaction of the borrow would be discarded by its own return - on the one engine
+ * where the database does not commit it for us. On postgres the per-column rows ANALYZE writes to
+ * {@code pg_statistic} are ordinary catalog rows and go with a rollback, taking the histogram the
+ * {@code where k>? order by k} batches of #859 need and leaving only the {@code pg_class.reltuples}
+ * that is written in place; mysql commits {@code ANALYZE TABLE} implicitly, {@code dbms_stats}
+ * commits of its own, and sql server does not roll its statistics update back either - which is
+ * why no container suite pins this, and why it is pinned here (issue #1012).
+ */
+ @Test
+ public void testEveryStatisticsStatementIsCommittedWhereItRuns() throws Exception {
+ final PreparedStatement statement = mock(PreparedStatement.class);
+ // postgres rather than the oracle stand-in above: it is the engine whose statement this commit
+ // is the only thing making durable
+ final Connection con = mock(postgresConnection.class); // the dialect is read off the connection
+ when(con.getNetworkTimeout()).thenReturn(0);
+ when(con.prepareStatement(anyString())).thenReturn(statement);
+
+ assertTrue(storage.updateTableStatistics(con, asList(
+ new TreeName("dc=example,dc=com", "id2entry"), new TreeName("dc=example,dc=com", "dn2id"))));
+
+ // Behind each statement and not once at the end of the loop: a single commit there would leave
+ // every tree but the last of a rebuild-index import inside the transaction of the borrow.
+ final InOrder perTree = inOrder(con, statement);
+ perTree.verify(statement).execute();
+ perTree.verify(con).commit();
+ perTree.verify(statement).execute();
+ perTree.verify(con).commit();
+ verify(con, times(2)).commit();
+ // the rollback of this loop belongs to its failure path: a refresh that went through must not
+ // end by throwing away what it just committed
+ verify(con, never()).rollback();
+ }
+
+ /**
* A connection whose driver refuses the call mid-flight is given back what it carried before.
* The entry holding that value is dropped as soon as the last statement on the connection is
* through, so a backstop left armed goes back to the pool as the connection's own read timeout
@@ -1441,4 +1479,7 @@
// this interface is an oracle connection as far as the storage is concerned - which is the
// whole reason for the lower case name here.
private interface oracleConnection extends Connection {}
+
+ /** The same trick for the engine whose statistics statement a rollback really would discard. */
+ private interface postgresConnection extends Connection {}
}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java b/opendj-server-legacy/src/test/java/org/opends/server/backends/jdbc/TestCase.java
index 2a002a8..75001d3 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
@@ -1866,8 +1866,17 @@
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 + "'";
+ // The per-column rows of pg_statistic rather than pg_class.reltuples, which is the half of
+ // what ANALYZE writes that cannot pin the refresh: reltuples is overwritten in place
+ // (vac_update_relstats(): "We violate transaction semantics here") and so survives the
+ // rollback of the connection's return, while these rows are ordinary catalog rows and do
+ // not - which is what makes the commit of updateTableStatistics() load-bearing on this
+ // engine (issue #1012). Strictly stronger than reltuples was: the count is 0 both for a
+ // table that was never analyzed and for one whose refresh was rolled back, and the row of
+ // pg_class still carries the query, so a table that is not there is still reported as
+ // "not found" rather than as stale statistics.
+ sql = "select (select count(*) from pg_statistic s where s.starelid=c.oid)"
+ + " from pg_class c where c.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() + "'";
--
Gitblit v1.10.0