Skip to content

Commit 7ce66b1

Browse files
geekypunkclaude
geekypunk
and
claude
committed
perf: fetch table indexes for a schema in one query, not one per table
`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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013dSdoA8UGq7PVyvEwWjPXM
1 parent 804e713 commit 7ce66b1

4 files changed

Lines changed: 281 additions & 3 deletions

File tree

‎backend/src/main/java/com/dbaagent/provider/api/IntrospectionProvider.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
import java.sql.Connection;
66
import java.sql.SQLException;
7+
import java.util.Collection;
8+
import java.util.HashMap;
79
import java.util.List;
10+
import java.util.Locale;
811
import java.util.Map;
912

1013
/**
@@ -48,6 +51,54 @@ public interface IntrospectionProvider {
4851
*/
4952
List<TableIndex> getTableIndexes(Connection connection, String database, String tableName) throws SQLException;
5053

54+
/**
55+
* Get indexes for every table in a schema in one round trip.
56+
*
57+
* <p>The per-table {@link #getTableIndexes} above is a round trip each, which turns
58+
* enrichment of a wide schema into hundreds of serial queries — painful on any link
59+
* with real latency (an SSH tunnel to a replica, say). This mirrors what
60+
* {@link #getForeignKeys} already does for constraints: fetch the whole schema once
61+
* and group in memory.
62+
*
63+
* <p>Results are keyed by the caller's own table name, lower-cased — whatever was
64+
* passed in, qualified or not — so a caller can look up what it asked for.
65+
*
66+
* <p>Two distinct outcomes, and callers must treat them differently:
67+
* <ul>
68+
* <li><b>Present, empty list</b> — the table was scanned and genuinely has no
69+
* indexes. Nothing further to do.</li>
70+
* <li><b>Absent</b> — this provider declined to answer for that name, and the
71+
* caller must fall back to {@link #getTableIndexes}. An implementation is free
72+
* to decline any name it cannot answer precisely; the Postgres one declines
73+
* schema-qualified names rather than risk merging indexes across schemas.</li>
74+
* </ul>
75+
*
76+
* <p>The default implementation just loops {@link #getTableIndexes}, so a provider
77+
* that does not override this behaves exactly as before.
78+
*
79+
* @param connection The database connection
80+
* @param database The database/schema name
81+
* @param tableNames Tables the caller cares about (used only by the default fallback)
82+
* @return Map of lower-cased caller-supplied table name to that table's indexes;
83+
* names the provider declined are absent rather than empty
84+
* @throws SQLException If a database error occurs
85+
*/
86+
default Map<String, List<TableIndex>> getAllTableIndexes(
87+
Connection connection, String database, Collection<String> tableNames
88+
) throws SQLException {
89+
Map<String, List<TableIndex>> byTable = new HashMap<>();
90+
for (String tableName : tableNames) {
91+
if (tableName == null) {
92+
continue;
93+
}
94+
byTable.put(
95+
tableName.toLowerCase(Locale.ROOT),
96+
getTableIndexes(connection, database, tableName)
97+
);
98+
}
99+
return byTable;
100+
}
101+
51102
/**
52103
* Get table statistics (size, row count, etc.).
53104
* @param connection The database connection

‎backend/src/main/java/com/dbaagent/provider/mysql/MySQLIntrospectionProvider.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,78 @@ public List<ColumnInfo> getTableColumns(Connection connection, String database,
148148
return columns;
149149
}
150150

151+
/**
152+
* One query for the whole schema instead of one per table.
153+
*
154+
* <p>INFORMATION_SCHEMA.STATISTICS is not cheap on MySQL, and paying for it 567 times
155+
* in a row across a tunnel is what made schema enrichment take minutes. Scoped to a
156+
* single TABLE_SCHEMA so this stays bounded on servers hosting many databases.
157+
*/
158+
@Override
159+
public Map<String, List<TableIndex>> getAllTableIndexes(
160+
Connection connection, String database, Collection<String> tableNames
161+
) throws SQLException {
162+
String query = """
163+
SELECT TABLE_NAME, INDEX_NAME, COLUMN_NAME, NON_UNIQUE, INDEX_TYPE, SEQ_IN_INDEX
164+
FROM INFORMATION_SCHEMA.STATISTICS
165+
WHERE TABLE_SCHEMA = ?
166+
ORDER BY TABLE_NAME, INDEX_NAME, SEQ_IN_INDEX
167+
""";
168+
169+
// table -> index name -> index, so multi-column indexes accumulate their columns
170+
// in SEQ_IN_INDEX order the same way the per-table path builds them.
171+
Map<String, Map<String, TableIndex>> byTable = new HashMap<>();
172+
173+
try (PreparedStatement stmt = connection.prepareStatement(query)) {
174+
stmt.setString(1, database);
175+
176+
try (ResultSet rs = stmt.executeQuery()) {
177+
while (rs.next()) {
178+
String tableName = rs.getString("TABLE_NAME");
179+
if (tableName == null) {
180+
continue;
181+
}
182+
String indexName = rs.getString("INDEX_NAME");
183+
String columnName = rs.getString("COLUMN_NAME");
184+
boolean nonUnique = rs.getBoolean("NON_UNIQUE");
185+
String indexType = rs.getString("INDEX_TYPE");
186+
187+
Map<String, TableIndex> indexMap =
188+
byTable.computeIfAbsent(tableName.toLowerCase(Locale.ROOT), k -> new LinkedHashMap<>());
189+
190+
TableIndex index = indexMap.get(indexName);
191+
if (index == null) {
192+
index = new TableIndex();
193+
index.setName(indexName);
194+
index.setType(indexType);
195+
index.setUnique(!nonUnique);
196+
index.setPrimary("PRIMARY".equals(indexName));
197+
index.setColumns(new ArrayList<>());
198+
indexMap.put(indexName, index);
199+
}
200+
index.getColumns().add(columnName);
201+
}
202+
}
203+
}
204+
205+
// Tables with no indexes at all must still be present, so callers can tell an
206+
// unindexed table from one this scan never covered.
207+
Map<String, List<TableIndex>> result = new HashMap<>();
208+
for (String tableName : tableNames) {
209+
if (tableName == null) {
210+
continue;
211+
}
212+
// Callers look up by the name they passed in, but STATISTICS returns bare
213+
// TABLE_NAMEs — so match on the bare name and key the result by the original.
214+
String key = tableName.toLowerCase(Locale.ROOT);
215+
int dot = key.lastIndexOf('.');
216+
String bare = dot > 0 ? key.substring(dot + 1) : key;
217+
Map<String, TableIndex> indexMap = byTable.get(bare);
218+
result.put(key, indexMap == null ? new ArrayList<>() : new ArrayList<>(indexMap.values()));
219+
}
220+
return result;
221+
}
222+
151223
@Override
152224
public List<TableIndex> getTableIndexes(Connection connection, String database, String tableName) throws SQLException {
153225
List<TableIndex> indexes = new ArrayList<>();

‎backend/src/main/java/com/dbaagent/provider/postgres/PostgresIntrospectionProvider.java‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,104 @@ LEFT JOIN (
211211
return columns;
212212
}
213213

214+
/**
215+
* One query for every requested table instead of one per table.
216+
*
217+
* <p>Same joins and filters as the per-table variant below; only the predicate
218+
* changes, from a single relname to an array of them. Matching on bare relname
219+
* across schemas is deliberate — it is exactly what the per-table path does for an
220+
* unqualified name, so this stays behaviour-preserving.
221+
*
222+
* <p>Schema-qualified names are declined (left out of the result) so the caller
223+
* falls back to the per-table query, which filters on schema. Matching them here
224+
* would be wrong either way: pg_class.relname is bare, so `s.t` matches nothing,
225+
* and stripping the qualifier would match that name in every schema and merge
226+
* their indexes.
227+
*/
228+
@Override
229+
public Map<String, List<TableIndex>> getAllTableIndexes(
230+
Connection connection, String database, Collection<String> tableNames
231+
) throws SQLException {
232+
String query = """
233+
SELECT
234+
t.relname AS table_name,
235+
i.relname AS index_name,
236+
a.attname AS column_name,
237+
ix.indisunique AS is_unique,
238+
ix.indisprimary AS is_primary,
239+
am.amname AS index_type
240+
FROM pg_class t
241+
JOIN pg_namespace n ON n.oid = t.relnamespace
242+
JOIN pg_index ix ON t.oid = ix.indrelid
243+
JOIN pg_class i ON i.oid = ix.indexrelid
244+
JOIN pg_attribute a ON a.attrelid = t.oid AND a.attnum = ANY(ix.indkey)
245+
JOIN pg_am am ON i.relam = am.oid
246+
WHERE t.relkind IN ('r', 'p', 'm', 'v')
247+
AND t.relname = ANY(?)
248+
ORDER BY t.relname, i.relname, a.attnum
249+
""";
250+
251+
Map<String, Map<String, TableIndex>> byTable = new HashMap<>();
252+
String[] names = tableNames.stream()
253+
.filter(Objects::nonNull)
254+
.toArray(String[]::new);
255+
256+
// Only unqualified names are answered here. relname is bare, so a `schema.table`
257+
// name matches nothing as written, and stripping the qualifier would be worse:
258+
// it would match that table name in *every* schema and merge their indexes. Such
259+
// names are simply left out of the result, which sends the caller to the
260+
// per-table path that filters on schema properly.
261+
String[] bareNames = Arrays.stream(names)
262+
.filter(n -> n.lastIndexOf('.') <= 0)
263+
.toArray(String[]::new);
264+
if (bareNames.length == 0) {
265+
return new HashMap<>();
266+
}
267+
268+
try (PreparedStatement stmt = connection.prepareStatement(query)) {
269+
stmt.setArray(1, connection.createArrayOf("text", bareNames));
270+
271+
try (ResultSet rs = stmt.executeQuery()) {
272+
while (rs.next()) {
273+
String tableName = rs.getString("table_name");
274+
if (tableName == null) {
275+
continue;
276+
}
277+
String indexName = rs.getString("index_name");
278+
String columnName = rs.getString("column_name");
279+
boolean isUnique = rs.getBoolean("is_unique");
280+
boolean isPrimary = rs.getBoolean("is_primary");
281+
String indexType = rs.getString("index_type");
282+
283+
Map<String, TableIndex> indexMap =
284+
byTable.computeIfAbsent(tableName.toLowerCase(Locale.ROOT), k -> new LinkedHashMap<>());
285+
286+
TableIndex index = indexMap.get(indexName);
287+
if (index == null) {
288+
index = new TableIndex();
289+
index.setName(indexName);
290+
index.setType(indexType);
291+
index.setUnique(isUnique);
292+
index.setPrimary(isPrimary);
293+
index.setColumns(new ArrayList<>());
294+
indexMap.put(indexName, index);
295+
}
296+
index.getColumns().add(columnName);
297+
}
298+
}
299+
}
300+
301+
// Every requested table gets an entry, so an unindexed table is distinguishable
302+
// from one this scan did not cover.
303+
Map<String, List<TableIndex>> result = new HashMap<>();
304+
for (String tableName : bareNames) {
305+
String key = tableName.toLowerCase(Locale.ROOT);
306+
Map<String, TableIndex> indexMap = byTable.get(key);
307+
result.put(key, indexMap == null ? new ArrayList<>() : new ArrayList<>(indexMap.values()));
308+
}
309+
return result;
310+
}
311+
214312
@Override
215313
public List<TableIndex> getTableIndexes(Connection connection, String database, String tableName) throws SQLException {
216314
List<TableIndex> indexes = new ArrayList<>();

‎backend/src/main/java/com/dbaagent/service/QueryExecutorService.java‎

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,41 @@ private void enrichColumnsWithKeyAndIndexMetadata(
170170
? loadForeignKeyColumns(connection, connRequest.getDatabase(), provider, connectionId)
171171
: Collections.emptySet();
172172

173+
// Indexes for every table up front, in one round trip. Fetching them
174+
// table-by-table inside the loop below meant one query per table — on a
175+
// 567-table schema behind a tunnel that was minutes, and it ran again for
176+
// every caller that missed the cache.
177+
//
178+
// Not every name is necessarily answered: a provider returns no entry for
179+
// one it cannot resolve precisely, and an object qualified with a database
180+
// other than this connection's is never offered in the first place. Either
181+
// way the loop below sees no entry and falls back to the per-table query,
182+
// which reads from the schema the name actually points at.
183+
Map<String, List<TableIndex>> indexesByTable = Collections.emptyMap();
184+
if (connection != null && provider != null) {
185+
List<String> bulkTables = new ArrayList<>();
186+
for (DatabaseObject obj : objects) {
187+
if (isBulkIndexable(obj, connRequest.getDatabase())) {
188+
bulkTables.add(obj.getName());
189+
}
190+
}
191+
if (!bulkTables.isEmpty()) {
192+
try {
193+
indexesByTable = provider.getAllTableIndexes(
194+
connection, connRequest.getDatabase(), bulkTables
195+
);
196+
} catch (Exception e) {
197+
// Fall back to the per-table path rather than losing index flags.
198+
log.warn(
199+
"Bulk index fetch failed for connection {} ({}); falling back to per-table",
200+
connectionId,
201+
e.getMessage()
202+
);
203+
indexesByTable = Collections.emptyMap();
204+
}
205+
}
206+
}
207+
173208
for (DatabaseObject obj : objects) {
174209
if (obj.getColumns() == null || obj.getColumns().isEmpty()) {
175210
continue;
@@ -181,9 +216,15 @@ private void enrichColumnsWithKeyAndIndexMetadata(
181216
&& obj.getType() != null
182217
&& "table".equalsIgnoreCase(obj.getType())) {
183218
try {
184-
List<TableIndex> indexes = provider.getTableIndexes(
185-
connection, connRequest.getDatabase(), obj.getName()
186-
);
219+
String key = obj.getName() == null
220+
? null
221+
: obj.getName().toLowerCase(Locale.ROOT);
222+
List<TableIndex> indexes = key == null ? null : indexesByTable.get(key);
223+
if (indexes == null) {
224+
indexes = provider.getTableIndexes(
225+
connection, connRequest.getDatabase(), obj.getName()
226+
);
227+
}
187228
applyIndexFlags(obj, indexes);
188229
} catch (Exception e) {
189230
log.debug(
@@ -240,6 +281,22 @@ private Set<String> loadForeignKeyColumns(
240281
return fkColumns;
241282
}
242283

284+
/**
285+
* True when the bulk (single-schema) index fetch can answer for this object.
286+
* A name qualified with a different schema must not be answered from the
287+
* connection database's index list.
288+
*/
289+
private boolean isBulkIndexable(DatabaseObject obj, String database) {
290+
if (obj == null || obj.getName() == null || !"table".equalsIgnoreCase(obj.getType())) {
291+
return false;
292+
}
293+
int dot = obj.getName().lastIndexOf('.');
294+
if (dot <= 0) {
295+
return true;
296+
}
297+
return obj.getName().substring(0, dot).equalsIgnoreCase(database);
298+
}
299+
243300
private void applyIndexFlags(DatabaseObject obj, List<TableIndex> indexes) {
244301
if (indexes == null || indexes.isEmpty()) {
245302
return;

0 commit comments

Comments
 (0)