-
Notifications
You must be signed in to change notification settings - Fork 18
init db and table, check health #55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,22 @@ const KEY_COLUMN: &str = "key"; | |
const VALUE_COLUMN: &str = "value"; | ||
const VERSION_COLUMN: &str = "version"; | ||
|
||
const DB_CHECK_STMT: &str = "SELECT 1 FROM pg_database WHERE datname = $1"; | ||
const DB_INIT_CMD: &str = "CREATE DATABASE"; | ||
const TABLE_CHECK_STMT: &str = "SELECT 1 FROM vss_db WHERE false"; | ||
const TABLE_INIT_STMT: &str = " | ||
CREATE TABLE IF NOT EXISTS vss_db ( | ||
user_token character varying(120) NOT NULL CHECK (user_token <> ''), | ||
store_id character varying(120) NOT NULL CHECK (store_id <> ''), | ||
key character varying(600) NOT NULL, | ||
value bytea NULL, | ||
version bigint NOT NULL, | ||
created_at TIMESTAMP WITH TIME ZONE, | ||
last_updated_at TIMESTAMP WITH TIME ZONE, | ||
PRIMARY KEY (user_token, store_id, key) | ||
); | ||
"; | ||
|
||
/// The maximum number of key versions that can be returned in a single page. | ||
/// | ||
/// This constant helps control memory and bandwidth usage for list operations, | ||
|
@@ -46,17 +62,103 @@ pub struct PostgresBackendImpl { | |
pool: Pool<PostgresConnectionManager<NoTls>>, | ||
} | ||
|
||
async fn initialize_vss_database(postgres_endpoint: &str, db_name: &str) -> Result<(), Error> { | ||
let postgres_dsn = format!("{}/{}", postgres_endpoint, "postgres"); | ||
let (client, connection) = tokio_postgres::connect(&postgres_dsn, NoTls).await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Connection error: {}", e)))?; | ||
// Connection must be driven on separate task, will be dropped on client dropped | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does it need to be driven on separate task? Should we still keep track of this connection handle somewhere? Rather than just printing once and giving up, should we handle the error and/or add a loop to retry the connection? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From the docs:
Is that satisfactory to you ?
The connection future will only resolve when I drop the client, so not sure what I would do with the associated
Since this is part of the startup sequence, I was leaning towards having very little error handling, and failing fast and early. This lets the user know that something is not right. Then once we are running, do all that we can to keep the show going. |
||
tokio::spawn(async move { | ||
if let Err(e) = connection.await { | ||
eprintln!("connection error: {}", e); | ||
} | ||
}); | ||
|
||
// Check if the database already exists | ||
let num_rows = client.execute(DB_CHECK_STMT, &[&db_name]).await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Query error: {}", e)))?; | ||
|
||
if num_rows == 0 { | ||
// Database does not exist, so create it | ||
let stmt = format!("{} {}", DB_INIT_CMD, db_name); | ||
client.execute(&stmt, &[]).await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Query error: {}", e)))?; | ||
println!("Database '{}' created successfully", db_name); | ||
} else { | ||
println!("Database '{}' already exists skipping creation", db_name); | ||
} | ||
|
||
Ok(()) | ||
} | ||
|
||
|
||
impl PostgresBackendImpl { | ||
/// Constructs a [`PostgresBackendImpl`] using `dsn` for PostgreSQL connection information. | ||
pub async fn new(dsn: &str) -> Result<Self, Error> { | ||
let manager = PostgresConnectionManager::new_from_stringlike(dsn, NoTls).map_err(|e| { | ||
pub async fn new(postgres_endpoint: &str, db_name: &str, init_db: bool) -> Result<Self, Error> { | ||
if init_db { | ||
tokio::time::timeout( | ||
tokio::time::Duration::from_secs(3), | ||
initialize_vss_database(postgres_endpoint, db_name), | ||
) | ||
.await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Is the postgres endpoint online? {}", e)))? | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Connection error: {}", e)))?; | ||
} | ||
let vss_dsn = format!("{}/{}", postgres_endpoint, db_name); | ||
let manager = PostgresConnectionManager::new_from_stringlike(vss_dsn, NoTls).map_err(|e| { | ||
Error::new(ErrorKind::Other, format!("Connection manager error: {}", e)) | ||
})?; | ||
let pool = Pool::builder() | ||
.build(manager) | ||
.await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Pool build error: {}", e)))?; | ||
Ok(PostgresBackendImpl { pool }) | ||
let ret = PostgresBackendImpl { pool }; | ||
let touch_table = async { | ||
if init_db { | ||
ret.initialize_vss_table().await?; | ||
} | ||
ret.check_health().await | ||
}; | ||
tokio::time::timeout( | ||
tokio::time::Duration::from_secs(3), | ||
touch_table, | ||
) | ||
.await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Does the database exist? If not use --init-db {}", e)))? | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Connection error: {}", e)))?; | ||
|
||
Ok(ret) | ||
} | ||
|
||
async fn initialize_vss_table(&self) -> Result<(), Error> { | ||
let conn = self | ||
.pool | ||
.get() | ||
.await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Connection error: {}", e)))?; | ||
let num_rows = conn | ||
.execute(TABLE_INIT_STMT, &[]) | ||
.await | ||
.map_err(|e| { | ||
Error::new(ErrorKind::Other, format!("Database operation failed. {}", e)) | ||
})?; | ||
assert_eq!(num_rows, 0); | ||
Ok(()) | ||
} | ||
|
||
async fn check_health(&self) -> Result<(), Error> { | ||
let conn = self | ||
.pool | ||
.get() | ||
.await | ||
.map_err(|e| Error::new(ErrorKind::Other, format!("Connection error: {}", e)))?; | ||
let num_rows = conn | ||
.execute(TABLE_CHECK_STMT, &[]) | ||
.await | ||
.map_err(|e| { | ||
Error::new(ErrorKind::Other, format!("Does the table exist? If not use --init-db {}", e)) | ||
})?; | ||
assert_eq!(num_rows, 0); | ||
Ok(()) | ||
} | ||
|
||
fn build_vss_record(&self, user_token: String, store_id: String, kv: KeyValue) -> VssDbRecord { | ||
|
@@ -413,7 +515,7 @@ mod tests { | |
define_kv_store_tests!( | ||
PostgresKvStoreTest, | ||
PostgresBackendImpl, | ||
PostgresBackendImpl::new("postgresql://postgres:postgres@localhost:5432/postgres") | ||
PostgresBackendImpl::new("postgresql://postgres:postgres@localhost:5432", "postgres", false) | ||
.await | ||
.unwrap() | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, do we want to expose a way for the user to configure their connection, e.g. to enable TLS?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes certainly, though may be out of scope for this PR.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed issue #56