From 7ce66b109e1e852a62dd0319e6aa0cbb815d3b19 Mon Sep 17 00:00:00 2001 From: Sasank Talasila Date: Thu, 27 Aug 2026 20:35:53 +0000 Subject: [PATCH] perf: fetch table indexes for a schema in one query, not one per table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `enrichColumnsWithKeyAndIndexMetadata` called `getTableIndexes` once per table inside its loop. On a wide schema that is hundreds of serial round trips, and it re-runs for every caller that misses the `databaseObjects` cache. Measured on a 567-table MySQL connection reached over an SSH tunnel, `GET /api/connections/{id}/objects`: before 152.66s / 196.30s on cache miss (~270ms per table) after 1.09s - 1.50s on cache miss cached 0.03-0.48s (unchanged) Both pre-fix requests were abandoned by the client (nginx 499). Because there is no stampede guard, a second caller arriving during the first sweep starts its own full sweep, so retrying made it worse. That stampede is left alone here: at ~1.5s it is no longer material, and adding locking to this path does not belong in a perf fix. The fix mirrors what `loadForeignKeyColumns` already does one line above: fetch the whole schema once and group in memory. Adds `IntrospectionProvider.getAllTableIndexes` with a default implementation that loops the existing per-table method, so a provider that does not override it is unchanged, plus overrides for both shipped providers: - MySQL: one INFORMATION_SCHEMA.STATISTICS query scoped to a single TABLE_SCHEMA, so it stays bounded on a server hosting many databases. Enrichment runs before the caller scopes results, so the requested set is already the whole schema and nothing extra is fetched. - Postgres: the same joins and filters as the per-table query, with `t.relname = ANY(?)` in place of `t.relname = ?`. The map distinguishes two outcomes, and this is load-bearing: a present but empty list means the table was scanned and has no indexes, while an absent key means the provider declined that name and the caller must fall back to `getTableIndexes`. Postgres declines schema-qualified names for that reason — pg_class.relname is bare, so `s.t` matches nothing, and stripping the qualifier would match that name in every schema and merge their indexes. The per-table query filters on schema and is the correct answer there. Behaviour preserved: - Objects qualified with a database other than the connection's are never offered to the bulk path. - A failed bulk fetch logs and falls back rather than dropping index flags. - Neither bulk query sets a statement timeout, matching the per-table methods they stand in for. A timeout here would be actively harmful: the fallback would then re-run the full N+1 on top of the time already spent. Verification. The Postgres bulk form was diffed against the per-table form on a live database using EXCEPT ALL in both directions: 744 rows each, zero rows differing either way, with composite index column order preserved. Both engines were exercised end to end with index flags still populated (MySQL: 760 columns with a single-column index and 375 composite, of 4588; Postgres: 32 and 2, of 128) and no fallback warnings logged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013dSdoA8UGq7PVyvEwWjPXM --- .../provider/api/IntrospectionProvider.java | 51 ++++++++++ .../mysql/MySQLIntrospectionProvider.java | 72 ++++++++++++++ .../PostgresIntrospectionProvider.java | 98 +++++++++++++++++++ .../service/QueryExecutorService.java | 63 +++++++++++- 4 files changed, 281 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/provider/api/IntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/api/IntrospectionProvider.java index 93c9dd3..7690bcc 100644 --- a/backend/src/main/java/com/dbaagent/provider/api/IntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/api/IntrospectionProvider.java @@ -4,7 +4,10 @@ import java.sql.Connection; import java.sql.SQLException; +import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; /** @@ -48,6 +51,54 @@ public interface IntrospectionProvider { */ List getTableIndexes(Connection connection, String database, String tableName) throws SQLException; + /** + * Get indexes for every table in a schema in one round trip. + * + *

The per-table {@link #getTableIndexes} above is a round trip each, which turns + * enrichment of a wide schema into hundreds of serial queries — painful on any link + * with real latency (an SSH tunnel to a replica, say). This mirrors what + * {@link #getForeignKeys} already does for constraints: fetch the whole schema once + * and group in memory. + * + *

Results are keyed by the caller's own table name, lower-cased — whatever was + * passed in, qualified or not — so a caller can look up what it asked for. + * + *

Two distinct outcomes, and callers must treat them differently: + *

    + *
  • Present, empty list — the table was scanned and genuinely has no + * indexes. Nothing further to do.
    • + *
  • Absent — this provider declined to answer for that name, and the + * caller must fall back to {@link #getTableIndexes}. An implementation is free + * to decline any name it cannot answer precisely; the Postgres one declines + * schema-qualified names rather than risk merging indexes across schemas.
    • + *
+ * + *

The default implementation just loops {@link #getTableIndexes}, so a provider + * that does not override this behaves exactly as before. + * + * @param connection The database connection + * @param database The database/schema name + * @param tableNames Tables the caller cares about (used only by the default fallback) + * @return Map of lower-cased caller-supplied table name to that table's indexes; + * names the provider declined are absent rather than empty + * @throws SQLException If a database error occurs + */ + default Map> getAllTableIndexes( + Connection connection, String database, Collection tableNames + ) throws SQLException { + Map> byTable = new HashMap<>(); + for (String tableName : tableNames) { + if (tableName == null) { + continue; + } + byTable.put( + tableName.toLowerCase(Locale.ROOT), + getTableIndexes(connection, database, tableName) + ); + } + return byTable; + } + /** * Get table statistics (size, row count, etc.). * @param connection The database connection diff --git a/backend/src/main/java/com/dbaagent/provider/mysql/MySQLIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/mysql/MySQLIntrospectionProvider.java index 39eb0a4..ca787bc 100644 --- a/backend/src/main/java/com/dbaagent/provider/mysql/MySQLIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/mysql/MySQLIntrospectionProvider.java @@ -148,6 +148,78 @@ public List getTableColumns(Connection connection, String database, return columns; } + /** + * One query for the whole schema instead of one per table. + * + *

INFORMATION_SCHEMA.STATISTICS is not cheap on MySQL, and paying for it 567 times + * in a row across a tunnel is what made schema enrichment take minutes. Scoped to a + * single TABLE_SCHEMA so this stays bounded on servers hosting many databases. + */ + @Override + public Map> getAllTableIndexes( + Connection connection, String database, Collection tableNames + ) throws SQLException { + String query = """ + SELECT TABLE_NAME, INDEX_NAME, COLUMN_NAME, NON_UNIQUE, INDEX_TYPE, SEQ_IN_INDEX + FROM INFORMATION_SCHEMA.STATISTICS + WHERE TABLE_SCHEMA = ? + ORDER BY TABLE_NAME, INDEX_NAME, SEQ_IN_INDEX + """; + + // table -> index name -> index, so multi-column indexes accumulate their columns + // in SEQ_IN_INDEX order the same way the per-table path builds them. + Map> byTable = new HashMap<>(); + + try (PreparedStatement stmt = connection.prepareStatement(query)) { + stmt.setString(1, database); + + try (ResultSet rs = stmt.executeQuery()) { + while (rs.next()) { + String tableName = rs.getString("TABLE_NAME"); + if (tableName == null) { + continue; + } + String indexName = rs.getString("INDEX_NAME"); + String columnName = rs.getString("COLUMN_NAME"); + boolean nonUnique = rs.getBoolean("NON_UNIQUE"); + String indexType = rs.getString("INDEX_TYPE"); + + Map indexMap = + byTable.computeIfAbsent(tableName.toLowerCase(Locale.ROOT), k -> new LinkedHashMap<>()); + + TableIndex index = indexMap.get(indexName); + if (index == null) { + index = new TableIndex(); + index.setName(indexName); + index.setType(indexType); + index.setUnique(!nonUnique); + index.setPrimary("PRIMARY".equals(indexName)); + index.setColumns(new ArrayList<>()); + indexMap.put(indexName, index); + } + index.getColumns().add(columnName); + } + } + } + + // Tables with no indexes at all must still be present, so callers can tell an + // unindexed table from one this scan never covered. + Map> result = new HashMap<>(); + for (String tableName : tableNames) { + if (tableName == null) { + continue; + } + // Callers look up by the name they passed in, but STATISTICS returns bare + // TABLE_NAMEs — so match on the bare name and key the result by the original. + String key = tableName.toLowerCase(Locale.ROOT); + int dot = key.lastIndexOf('.'); + String bare = dot > 0 ? key.substring(dot + 1) : key; + Map indexMap = byTable.get(bare); + result.put(key, indexMap == null ? new ArrayList<>() : new ArrayList<>(indexMap.values())); + } + return result; + } + @Override public List getTableIndexes(Connection connection, String database, String tableName) throws SQLException { List indexes = new ArrayList<>(); diff --git a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java index 68eb646..9d4eebc 100644 --- a/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java +++ b/backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java @@ -211,6 +211,104 @@ LEFT JOIN ( return columns; } + /** + * One query for every requested table instead of one per table. + * + *

Same joins and filters as the per-table variant below; only the predicate + * changes, from a single relname to an array of them. Matching on bare relname + * across schemas is deliberate — it is exactly what the per-table path does for an + * unqualified name, so this stays behaviour-preserving. + * + *

Schema-qualified names are declined (left out of the result) so the caller + * falls back to the per-table query, which filters on schema. Matching them here + * would be wrong either way: pg_class.relname is bare, so `s.t` matches nothing, + * and stripping the qualifier would match that name in every schema and merge + * their indexes. + */ + @Override + public Map> getAllTableIndexes( + Connection connection, String database, Collection tableNames + ) throws SQLException { + String query = """ + SELECT + t.relname AS table_name, + i.relname AS index_name, + a.attname AS column_name, + ix.indisunique AS is_unique, + ix.indisprimary AS is_primary, + am.amname AS index_type + FROM pg_class t + JOIN pg_namespace n ON n.oid = t.relnamespace + JOIN pg_index ix ON t.oid = ix.indrelid + JOIN pg_class i ON i.oid = ix.indexrelid + JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey) + JOIN pg_am am ON i.relam = am.oid + WHERE t.relkind IN ('r', 'p', 'm', 'v') + AND t.relname = ANY(?) + ORDER BY t.relname, i.relname, a.attnum + """; + + Map> byTable = new HashMap<>(); + String[] names = tableNames.stream() + .filter(Objects::nonNull) + .toArray(String[]::new); + + // Only unqualified names are answered here. relname is bare, so a `schema.table` + // name matches nothing as written, and stripping the qualifier would be worse: + // it would match that table name in *every* schema and merge their indexes. Such + // names are simply left out of the result, which sends the caller to the + // per-table path that filters on schema properly. + String[] bareNames = Arrays.stream(names) + .filter(n -> n.lastIndexOf('.') <= 0) + .toArray(String[]::new); + if (bareNames.length == 0) { + return new HashMap<>(); + } + + try (PreparedStatement stmt = connection.prepareStatement(query)) { + stmt.setArray(1, connection.createArrayOf("text", bareNames)); + + try (ResultSet rs = stmt.executeQuery()) { + while (rs.next()) { + String tableName = rs.getString("table_name"); + if (tableName == null) { + continue; + } + String indexName = rs.getString("index_name"); + String columnName = rs.getString("column_name"); + boolean isUnique = rs.getBoolean("is_unique"); + boolean isPrimary = rs.getBoolean("is_primary"); + String indexType = rs.getString("index_type"); + + Map indexMap = + byTable.computeIfAbsent(tableName.toLowerCase(Locale.ROOT), k -> new LinkedHashMap<>()); + + TableIndex index = indexMap.get(indexName); + if (index == null) { + index = new TableIndex(); + index.setName(indexName); + index.setType(indexType); + index.setUnique(isUnique); + index.setPrimary(isPrimary); + index.setColumns(new ArrayList<>()); + indexMap.put(indexName, index); + } + index.getColumns().add(columnName); + } + } + } + + // Every requested table gets an entry, so an unindexed table is distinguishable + // from one this scan did not cover. + Map> result = new HashMap<>(); + for (String tableName : bareNames) { + String key = tableName.toLowerCase(Locale.ROOT); + Map indexMap = byTable.get(key); + result.put(key, indexMap == null ? new ArrayList<>() : new ArrayList<>(indexMap.values())); + } + return result; + } + @Override public List getTableIndexes(Connection connection, String database, String tableName) throws SQLException { List indexes = new ArrayList<>(); diff --git a/backend/src/main/java/com/dbaagent/service/QueryExecutorService.java b/backend/src/main/java/com/dbaagent/service/QueryExecutorService.java index 110eaa7..ff2d55d 100644 --- a/backend/src/main/java/com/dbaagent/service/QueryExecutorService.java +++ b/backend/src/main/java/com/dbaagent/service/QueryExecutorService.java @@ -170,6 +170,41 @@ private void enrichColumnsWithKeyAndIndexMetadata( ? loadForeignKeyColumns(connection, connRequest.getDatabase(), provider, connectionId) : Collections.emptySet(); + // Indexes for every table up front, in one round trip. Fetching them + // table-by-table inside the loop below meant one query per table — on a + // 567-table schema behind a tunnel that was minutes, and it ran again for + // every caller that missed the cache. + // + // Not every name is necessarily answered: a provider returns no entry for + // one it cannot resolve precisely, and an object qualified with a database + // other than this connection's is never offered in the first place. Either + // way the loop below sees no entry and falls back to the per-table query, + // which reads from the schema the name actually points at. + Map> indexesByTable = Collections.emptyMap(); + if (connection != null && provider != null) { + List bulkTables = new ArrayList<>(); + for (DatabaseObject obj : objects) { + if (isBulkIndexable(obj, connRequest.getDatabase())) { + bulkTables.add(obj.getName()); + } + } + if (!bulkTables.isEmpty()) { + try { + indexesByTable = provider.getAllTableIndexes( + connection, connRequest.getDatabase(), bulkTables + ); + } catch (Exception e) { + // Fall back to the per-table path rather than losing index flags. + log.warn( + "Bulk index fetch failed for connection {} ({}); falling back to per-table", + connectionId, + e.getMessage() + ); + indexesByTable = Collections.emptyMap(); + } + } + } + for (DatabaseObject obj : objects) { if (obj.getColumns() == null || obj.getColumns().isEmpty()) { continue; @@ -181,9 +216,15 @@ private void enrichColumnsWithKeyAndIndexMetadata( && obj.getType() != null && "table".equalsIgnoreCase(obj.getType())) { try { - List indexes = provider.getTableIndexes( - connection, connRequest.getDatabase(), obj.getName() - ); + String key = obj.getName() == null + ? null + : obj.getName().toLowerCase(Locale.ROOT); + List indexes = key == null ? null : indexesByTable.get(key); + if (indexes == null) { + indexes = provider.getTableIndexes( + connection, connRequest.getDatabase(), obj.getName() + ); + } applyIndexFlags(obj, indexes); } catch (Exception e) { log.debug( @@ -240,6 +281,22 @@ private Set loadForeignKeyColumns( return fkColumns; } + /** + * True when the bulk (single-schema) index fetch can answer for this object. + * A name qualified with a different schema must not be answered from the + * connection database's index list. + */ + private boolean isBulkIndexable(DatabaseObject obj, String database) { + if (obj == null || obj.getName() == null || !"table".equalsIgnoreCase(obj.getType())) { + return false; + } + int dot = obj.getName().lastIndexOf('.'); + if (dot <= 0) { + return true; + } + return obj.getName().substring(0, dot).equalsIgnoreCase(database); + } + private void applyIndexFlags(DatabaseObject obj, List indexes) { if (indexes == null || indexes.isEmpty()) { return;