Skip to content

Commit c5e7bab

Browse files
committed
Lock Postgres stores on initialization
Prevent multiple nodes from opening the same PostgreSQL database table at once while allowing separate database and table pairs to coexist. Retain the session-scoped advisory lock for the store lifetime. This change was created with OpenAI Codex.
1 parent abbed3e commit c5e7bab

2 files changed

Lines changed: 71 additions & 3 deletions

File tree

src/builder.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,8 @@ impl NodeBuilder {
695695
///
696696
/// The given `kv_table_name` will be used or default to
697697
/// [`DEFAULT_KV_TABLE_NAME`](io::postgres_store::DEFAULT_KV_TABLE_NAME).
698+
/// Building fails if another PostgreSQL-backed node using the same database and table is still
699+
/// alive. Nodes using a different database or table on the same server may coexist.
698700
///
699701
/// If `certificate_pem` is `Some`, TLS will be used for database connections and the
700702
/// provided PEM-encoded CA certificate will be added to the system's default root
@@ -1229,6 +1231,8 @@ impl ArcedNodeBuilder {
12291231
///
12301232
/// The given `kv_table_name` will be used or default to
12311233
/// [`DEFAULT_KV_TABLE_NAME`](io::postgres_store::DEFAULT_KV_TABLE_NAME).
1234+
/// Building fails if another PostgreSQL-backed node using the same database and table is still
1235+
/// alive. Nodes using a different database or table on the same server may coexist.
12321236
///
12331237
/// If `certificate_pem` is `Some`, TLS will be used for database connections and the
12341238
/// provided PEM-encoded CA certificate will be added to the system's default root

src/io/postgres_store/mod.rs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::future::Future;
1111
use std::sync::atomic::{AtomicU64, Ordering};
1212
use std::sync::{Arc, Mutex};
1313

14+
use bitcoin::hashes::{sha256, Hash, HashEngine};
1415
use lightning::io;
1516
use lightning::util::persist::{
1617
KVStore, MigratableKVStore, PageToken, PaginatedKVStore, PaginatedListResponse,
@@ -44,6 +45,18 @@ const PAGE_SIZE: usize = 50;
4445
// Keep this small while still allowing progress if one runtime worker blocks on sync store access.
4546
const INTERNAL_RUNTIME_WORKERS: usize = 2;
4647

48+
fn advisory_lock_id(db_name: &str, kv_table_name: &str) -> i64 {
49+
let mut engine = sha256::Hash::engine();
50+
engine.input(b"ldk-node:postgres-store");
51+
for component in [db_name, kv_table_name] {
52+
engine.input(&(component.len() as u64).to_be_bytes());
53+
engine.input(component.as_bytes());
54+
}
55+
56+
let hash = sha256::Hash::from_engine(engine).to_byte_array();
57+
i64::from_be_bytes(hash[..8].try_into().expect("SHA-256 prefix has the expected length"))
58+
}
59+
4760
fn sql_identifier(identifier: &str) -> io::Result<String> {
4861
if identifier.is_empty() || identifier.contains('\0') {
4962
return Err(io::Error::new(
@@ -128,6 +141,8 @@ impl PostgresStore {
128141
/// the default `postgres` database to create it.
129142
///
130143
/// The given `kv_table_name` will be used or default to [`DEFAULT_KV_TABLE_NAME`].
144+
/// Construction fails if another [`PostgresStore`] using the same database and table is still
145+
/// alive. Stores using a different database or table on the same PostgreSQL server may coexist.
131146
///
132147
/// If `certificate_pem` is `Some`, TLS will be used for database connections and the
133148
/// provided PEM-encoded CA certificate will be added to the system's default root
@@ -373,6 +388,9 @@ impl MigratableKVStore for PostgresStore {
373388

374389
struct PostgresStoreInner {
375390
pool: SmallPool,
391+
// PostgreSQL advisory locks are session-scoped, so keep the connection that acquired our lock
392+
// alive for the lifetime of the store.
393+
_lock_client: ClientConnection,
376394
config: Config,
377395
kv_table_name_sql: String,
378396
tls: PgTlsConnector,
@@ -426,6 +444,23 @@ impl PostgresStoreInner {
426444
Self::create_database_if_not_exists(&config, &tls, logger.as_deref()).await?;
427445

428446
let client = make_config_connection(&config, &tls).await?;
447+
let lock_id = advisory_lock_id(&db_name, &kv_table_name);
448+
let row = client.query_one("SELECT pg_try_advisory_lock($1)", &[&lock_id]).await.map_err(
449+
|e| {
450+
let msg = format!(
451+
"Failed to acquire PostgreSQL store lock for database {db_name} and table {kv_table_name}: {e}"
452+
);
453+
io::Error::new(io::ErrorKind::Other, msg)
454+
},
455+
)?;
456+
if !row.get::<_, bool>(0) {
457+
return Err(io::Error::new(
458+
io::ErrorKind::AlreadyExists,
459+
format!(
460+
"PostgreSQL store for database {db_name} and table {kv_table_name} is already in use"
461+
),
462+
));
463+
}
429464

430465
// Create the KV data table if it doesn't exist. `sort_order` uses BIGSERIAL so
431466
// the database assigns a fresh, monotonically increasing value on each INSERT and
@@ -502,12 +537,18 @@ impl PostgresStoreInner {
502537
io::Error::new(io::ErrorKind::Other, msg)
503538
})?;
504539

505-
// Drop the setup client; the pool builds its own POOL_SIZE fresh connections.
506-
drop(client);
507540
let pool = SmallPool::new(&config, &tls).await?;
508541

509542
let write_version_locks = Mutex::new(HashMap::new());
510-
Ok(Self { pool, config, kv_table_name_sql, tls, write_version_locks, logger })
543+
Ok(Self {
544+
pool,
545+
_lock_client: client,
546+
config,
547+
kv_table_name_sql,
548+
tls,
549+
write_version_locks,
550+
logger,
551+
})
511552
}
512553

513554
async fn create_database_if_not_exists(
@@ -927,6 +968,29 @@ mod tests {
927968
assert!(sql_table_identifier("schema.").is_err());
928969
}
929970

971+
#[test]
972+
fn test_postgres_advisory_lock_id_uses_database_and_table() {
973+
let lock_id = advisory_lock_id("database_a", "table_a");
974+
assert_eq!(lock_id, advisory_lock_id("database_a", "table_a"));
975+
assert_ne!(lock_id, advisory_lock_id("database_b", "table_a"));
976+
assert_ne!(lock_id, advisory_lock_id("database_a", "table_b"));
977+
}
978+
979+
#[tokio::test(flavor = "multi_thread")]
980+
async fn test_postgres_store_advisory_lock() {
981+
let table_name = "test_pg_advisory_lock";
982+
let store = create_test_store(table_name).await;
983+
984+
let err =
985+
PostgresStore::new(test_connection_string(), None, Some(table_name.to_string()), None)
986+
.await
987+
.err()
988+
.expect("a second store using the same database and table must fail");
989+
assert_eq!(err.kind(), io::ErrorKind::AlreadyExists);
990+
991+
cleanup_store(&store).await;
992+
}
993+
930994
#[tokio::test(flavor = "multi_thread")]
931995
async fn read_write_remove_list_persist() {
932996
let store = create_test_store("test_rwrl").await;

0 commit comments

Comments
 (0)