Skip to content

Commit 1c329de

Browse files
authored
perf(replica): reuse postgres connections in schema migration prep (#54)
2 parents a1b3213 + 7ff4f0c commit 1c329de

2 files changed

Lines changed: 91 additions & 114 deletions

File tree

src/controllers/postgres.rs

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::collections::HashSet;
2+
13
use k8s_openapi::api::core::v1::{Pod, Secret};
24
use kube::{
35
Api, Client, ResourceExt,
@@ -219,6 +221,19 @@ pub async fn discover_restore_database(
219221
}
220222
}
221223

224+
/// Query the on-disk size of the current database on an already-open
225+
/// connection. The connection-opening variant is `measure_database_size`,
226+
/// kept for callers that only need this single query; prefer this when
227+
/// you're going to make multiple queries on the same database so the
228+
/// connection can be reused.
229+
pub async fn database_size_on(pg: &tokio_postgres::Client) -> Result<u64> {
230+
let row = pg
231+
.query_one("SELECT pg_database_size(current_database())", &[])
232+
.await?;
233+
let size: i64 = row.get(0);
234+
Ok(size as u64)
235+
}
236+
222237
/// Query the on-disk size of the given database (bytes) via `pg_database_size()`.
223238
pub async fn measure_database_size(
224239
client: &Client,
@@ -239,21 +254,48 @@ pub async fn measure_database_size(
239254
use_port_forward,
240255
)
241256
.await?;
242-
243-
let row = conn
244-
.client
245-
.query_one("SELECT pg_database_size(current_database())", &[])
246-
.await?;
247-
248-
let size: i64 = row.get(0);
249-
Ok(size as u64)
257+
database_size_on(&conn.client).await
250258
}
251259

252260
/// Escape a SQL identifier by double-quoting it.
253261
pub fn quote_ident(s: &str) -> String {
254262
format!("\"{}\"", s.replace('"', "\"\""))
255263
}
256264

265+
/// Return the subset of `candidates` that exist as schemas in the
266+
/// database the connection is bound to. Reusable on an open connection.
267+
pub async fn existing_schemas_on(
268+
pg: &tokio_postgres::Client,
269+
candidates: &[String],
270+
) -> Result<Vec<String>> {
271+
if candidates.is_empty() {
272+
return Ok(Vec::new());
273+
}
274+
let rows = pg
275+
.query(
276+
"SELECT nspname FROM pg_catalog.pg_namespace WHERE nspname = ANY($1)",
277+
&[&candidates],
278+
)
279+
.await?;
280+
let found: HashSet<String> = rows.iter().map(|r| r.get::<_, String>(0)).collect();
281+
Ok(candidates
282+
.iter()
283+
.filter(|s| found.contains(s.as_str()))
284+
.cloned()
285+
.collect())
286+
}
287+
288+
/// Drop the given schemas (and all contents) on an already-open connection.
289+
/// Idempotent: missing schemas are skipped via `DROP SCHEMA IF EXISTS`.
290+
pub async fn drop_schemas_on(pg: &tokio_postgres::Client, schemas: &[String]) -> Result<()> {
291+
for schema in schemas {
292+
let stmt = format!("DROP SCHEMA IF EXISTS {} CASCADE", quote_ident(schema));
293+
debug!(schema = schema, "dropping schema");
294+
pg.execute(stmt.as_str(), &[]).await?;
295+
}
296+
Ok(())
297+
}
298+
257299
/// Drop the given schemas (and all their contents) from the named database
258300
/// in the restore. Idempotent: schemas that do not exist are silently
259301
/// skipped via `DROP SCHEMA IF EXISTS`. Used to wipe persistent_schemas in
@@ -290,12 +332,7 @@ pub async fn drop_schemas_in_restore(
290332
use_port_forward,
291333
)
292334
.await?;
293-
for schema in schemas {
294-
let stmt = format!("DROP SCHEMA IF EXISTS {} CASCADE", quote_ident(schema));
295-
debug!(restore = restore_name, schema = schema, "dropping schema");
296-
conn.client.execute(stmt.as_str(), &[]).await?;
297-
}
298-
Ok(())
335+
drop_schemas_on(&conn.client, schemas).await
299336
}
300337

301338
#[cfg(test)]

src/controllers/replica.rs

Lines changed: 40 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -1279,20 +1279,26 @@ async fn reconcile_schema_migration(
12791279
Err(e) => return Err(e),
12801280
};
12811281

1282-
// Filter out schemas that don't exist on the source. This happens when the
1283-
// user adds a schema to persistent_schemas before actually creating it.
1282+
// Source-side queries: schema existence + database size. Both run on
1283+
// (source restore, source_dbname), so we open one connection and
1284+
// reuse it across both queries to halve the connection count for
1285+
// this leg of the reconcile.
12841286
let all_schemas: &[String] = schemas;
1285-
let schemas = query_existing_schemas(
1286-
client,
1287-
namespace,
1288-
&old_restore_name,
1289-
&source_dbname,
1290-
&reader_user,
1291-
&reader_password,
1292-
all_schemas,
1293-
ctx.use_port_forward(),
1294-
)
1295-
.await?;
1287+
let (schemas, db_size_bytes) = {
1288+
let conn = postgres::connect_to_restore(
1289+
client,
1290+
namespace,
1291+
&old_restore_name,
1292+
&source_dbname,
1293+
&reader_user,
1294+
&reader_password,
1295+
ctx.use_port_forward(),
1296+
)
1297+
.await?;
1298+
let existing = postgres::existing_schemas_on(&conn.client, all_schemas).await?;
1299+
let size = postgres::database_size_on(&conn.client).await?;
1300+
(existing, size)
1301+
};
12961302
let existing_set: HashSet<&str> = schemas.iter().map(String::as_str).collect();
12971303
let skipped: Vec<&String> = all_schemas
12981304
.iter()
@@ -1315,21 +1321,6 @@ async fn reconcile_schema_migration(
13151321
return Ok(true);
13161322
}
13171323

1318-
// Measure the actual on-disk database size of the source restore and
1319-
// compute how much the persistent schemas have grown beyond the original
1320-
// snapshot. This delta is stored in the replica status so the next
1321-
// restore PVC can be sized accordingly.
1322-
let db_size_bytes = postgres::measure_database_size(
1323-
client,
1324-
namespace,
1325-
&old_restore_name,
1326-
&source_dbname,
1327-
&reader_user,
1328-
&reader_password,
1329-
ctx.use_port_forward(),
1330-
)
1331-
.await?;
1332-
13331324
let snapshot_size_bytes: ParsedQuantity = old_restore
13341325
.spec
13351326
.snapshot_size
@@ -1394,42 +1385,36 @@ async fn reconcile_schema_migration(
13941385
Err(e) => return Err(e),
13951386
};
13961387

1397-
// Drop any persistent_schemas that are already present in the new
1398-
// restore. They are operator-owned: whatever was in the restored data
1399-
// is junk (typically because the schema has leaked back into the source
1400-
// DB), and the migration Job is about to write the canonical copy
1401-
// from the previous restore via pg_dump | psql, which would otherwise
1402-
// conflict on existing objects. Idempotent and no-op when the schemas
1403-
// are absent — safe to run unconditionally before every migration.
1404-
let preexisting = query_existing_schemas(
1405-
client,
1406-
namespace,
1407-
&new_restore_name,
1408-
&target_dbname,
1409-
&reader_user,
1410-
&reader_password,
1411-
&schemas,
1412-
ctx.use_port_forward(),
1413-
)
1414-
.await?;
1415-
if !preexisting.is_empty() {
1416-
info!(
1417-
replica = %replica_name,
1418-
restore = %new_restore_name,
1419-
schemas = ?preexisting,
1420-
"dropping pre-existing persistent schemas in target before migration"
1421-
);
1422-
postgres::drop_schemas_in_restore(
1388+
// Target-side: detect any persistent_schemas that are already present
1389+
// in the new restore and drop them. Both queries run on (new restore,
1390+
// target_dbname), so open one connection and reuse it for both. The
1391+
// schemas to drop are operator-owned — whatever was in the restored
1392+
// data is junk (typically because the schema has leaked back into
1393+
// the source DB) — and the migration Job is about to write the
1394+
// canonical copy from the previous restore via pg_dump | psql,
1395+
// which would otherwise conflict on existing objects. Idempotent
1396+
// and no-op when the schemas are absent.
1397+
{
1398+
let conn = postgres::connect_to_restore(
14231399
client,
14241400
namespace,
14251401
&new_restore_name,
14261402
&target_dbname,
14271403
&reader_user,
14281404
&reader_password,
1429-
&preexisting,
14301405
ctx.use_port_forward(),
14311406
)
14321407
.await?;
1408+
let preexisting = postgres::existing_schemas_on(&conn.client, &schemas).await?;
1409+
if !preexisting.is_empty() {
1410+
info!(
1411+
replica = %replica_name,
1412+
restore = %new_restore_name,
1413+
schemas = ?preexisting,
1414+
"dropping pre-existing persistent schemas in target before migration"
1415+
);
1416+
postgres::drop_schemas_on(&conn.client, &preexisting).await?;
1417+
}
14331418
}
14341419

14351420
// The analytics user is granted SUPERUSER on every read-write restore
@@ -1482,51 +1467,6 @@ async fn reconcile_schema_migration(
14821467
Ok(false) // Job created, not complete yet
14831468
}
14841469

1485-
/// Query which of the requested schemas actually exist in a restore's database.
1486-
/// Returns schemas in the same order as the input slice, using the system
1487-
/// catalog (`pg_catalog.pg_namespace`) which is always fully visible regardless
1488-
/// of schema-level privileges.
1489-
#[expect(
1490-
clippy::too_many_arguments,
1491-
reason = "internal helper with tightly-coupled params"
1492-
)]
1493-
async fn query_existing_schemas(
1494-
client: &Client,
1495-
namespace: &str,
1496-
restore_name: &str,
1497-
dbname: &str,
1498-
user: &str,
1499-
password: &str,
1500-
schemas: &[String],
1501-
use_port_forward: bool,
1502-
) -> Result<Vec<String>> {
1503-
let conn = postgres::connect_to_restore(
1504-
client,
1505-
namespace,
1506-
restore_name,
1507-
dbname,
1508-
user,
1509-
password,
1510-
use_port_forward,
1511-
)
1512-
.await?;
1513-
1514-
let rows = conn
1515-
.client
1516-
.query(
1517-
"SELECT nspname FROM pg_catalog.pg_namespace WHERE nspname = ANY($1)",
1518-
&[&schemas],
1519-
)
1520-
.await?;
1521-
1522-
let found: HashSet<String> = rows.iter().map(|r| r.get::<_, String>(0)).collect();
1523-
Ok(schemas
1524-
.iter()
1525-
.filter(|s| found.contains(s.as_str()))
1526-
.cloned()
1527-
.collect())
1528-
}
1529-
15301470
pub fn error_policy(
15311471
_replica: Arc<PostgresPhysicalReplica>,
15321472
error: &Error,

0 commit comments

Comments
 (0)