Skip to content

Fix container healthcheck #245

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.

Sign up for GitHub

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

Open
feelingsonice wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix container healthcheck #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/cornucopia/src/container.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use self::error::Error;
/// Starts Cornucopia's database container and wait until it reports healthy.
pub fn setup(podman: bool) -> Result<(), Error> {
spawn_container(podman)?;
healthcheck(podman, 120, 50)?;
Copy link

@Gerald-Hu Gerald-Hu Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why too quick is a trouble? And do we really want to 10x it?

Copy link
Author

@feelingsonice feelingsonice Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This health check is pinging the Postgres container as it starts up, because the container needs sometime to start, ping it too quickly means the container won't be ready yet.

Copy link

@heksesang heksesang May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it retries the pings, why would it be a problem that the container isn't ready yet?

Copy link
Author

@feelingsonice feelingsonice May 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the health checker is erroring out before it could do any retry:

fn healthcheck(podman: bool, max_retries: u64, ms_per_retry: u64) -> Result<(), Error> {
    let slow_threshold = 10 + max_retries / 10;
    let mut nb_retries = 0;
    while !is_postgres_healthy(podman)? { // <------ this won't execute for me
       
    }
    // ...
}

I haven't confirmed this but a quick Look suggests that is_postgres_healthy is calling a command on the cornucopia_postgres container before it's even created:

/// Checks if Cornucopia's container reports healthy
fn is_postgres_healthy(podman: bool) -> Result<bool, Error> {
    Ok(cmd(
        podman,
        &["exec", "cornucopia_postgres", "pg_isready"],
        "check container health",
    )
    .is_ok())
}

Copy link

@heksesang heksesang May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is_postgres_healthy can only return an Ok(true) or Ok(false), so it cannot actually error out?

healthcheck(podman, 120, 500)?;
Ok(())
}

Expand Down
5 changes: 4 additions & 1 deletion crates/cornucopia/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ pub fn generate_managed<P: AsRef<Path>>(
.into_iter()
.map(parse_query_module)
.collect::<Result<_, parser::error::Error>>()?;
container::setup(podman)?;
if let Err(e) = container::setup(podman) {
let _ = container::cleanup(podman);
return Err(Error::Container(e));
}
let mut client = conn::cornucopia_conn()?;
load_schema(&mut client, schema_files)?;
let prepared_modules = prepare(&mut client, modules)?;
Expand Down
5 changes: 3 additions & 2 deletions examples/auto_build/_build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ fn main() -> Result<(), Error> {
let schema_file = "schema.sql";
let destination = "src/cornucopia.rs";
let settings = CodegenSettings {
is_async: true,
derive_ser: false,
gen_async: true,
gen_sync: false,
derive_ser: true,
};

println!("cargo:rerun-if-changed={queries_path}");
Expand Down
141 changes: 55 additions & 86 deletions examples/auto_build/src/cornucopia.rs
View file
Open in desktop
Copy link

@Gerald-Hu Gerald-Hu Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can barely understand this file and I hate lifecycle lol... Is this a generated file?

Copy link
Author

@feelingsonice feelingsonice Apr 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its auto generated.

Original file line number Diff line number Diff line change
@@ -1,93 +1,62 @@
// This file was generated with `cornucopia`. Do not modify.

#[allow(clippy::all, clippy::pedantic)]
#[allow(unused_variables)]
#[allow(unused_imports)]
#[allow(dead_code)]
pub mod types {}
#[allow(clippy::all, clippy::pedantic)]
#[allow(unused_variables)]
#[allow(unused_imports)]
#[allow(dead_code)]
pub mod queries {
pub mod module_1 {
use cornucopia_async::GenericClient;
use futures;
use futures::{StreamExt, TryStreamExt};
pub struct StringQuery<'a, C: GenericClient, T, const N: usize> {
client: &'a C,
params: [&'a (dyn postgres_types::ToSql + Sync); N],
stmt: &'a mut cornucopia_async::private::Stmt,
extractor: fn(&tokio_postgres::Row) -> &str,
mapper: fn(&str) -> T,
}
impl<'a, C, T: 'a, const N: usize> StringQuery<'a, C, T, N>
where
C: GenericClient,
#[allow(clippy::all, clippy::pedantic)] #[allow(unused_variables)]
#[allow(unused_imports)] #[allow(dead_code)] pub mod types { } #[allow(clippy::all, clippy::pedantic)] #[allow(unused_variables)]
#[allow(unused_imports)] #[allow(dead_code)] pub mod queries
{ pub mod module_1
{ use futures::{StreamExt, TryStreamExt};use futures; use cornucopia_async::GenericClient; pub struct StringQuery<'a, C: GenericClient, T, const N: usize>
{
client: &'a C, params:
[&'a (dyn postgres_types::ToSql + Sync); N], stmt: &'a mut
cornucopia_async::private::Stmt, extractor: fn(&tokio_postgres::Row) -> & str,
mapper: fn(& str) -> T,
} impl<'a, C, T:'a, const N: usize> StringQuery<'a, C, T, N> where C:
GenericClient
{
pub fn map<R>(self, mapper: fn(& str) -> R) ->
StringQuery<'a,C,R,N>
{
StringQuery
{
pub fn map<R>(self, mapper: fn(&str) -> R) -> StringQuery<'a, C, R, N> {
StringQuery {
client: self.client,
params: self.params,
stmt: self.stmt,
extractor: self.extractor,
mapper,
}
}
pub async fn one(self) -> Result<T, tokio_postgres::Error> {
let stmt = self.stmt.prepare(self.client).await?;
let row = self.client.query_one(stmt, &self.params).await?;
Ok((self.mapper)((self.extractor)(&row)))
}
pub async fn all(self) -> Result<Vec<T>, tokio_postgres::Error> {
self.iter().await?.try_collect().await
}
pub async fn opt(self) -> Result<Option<T>, tokio_postgres::Error> {
let stmt = self.stmt.prepare(self.client).await?;
Ok(self
.client
.query_opt(stmt, &self.params)
.await?
.map(|row| (self.mapper)((self.extractor)(&row))))
}
pub async fn iter(
self,
) -> Result<
impl futures::Stream<Item = Result<T, tokio_postgres::Error>> + 'a,
tokio_postgres::Error,
> {
let stmt = self.stmt.prepare(self.client).await?;
let it = self
.client
.query_raw(stmt, cornucopia_async::private::slice_iter(&self.params))
.await?
.map(move |res| res.map(|row| (self.mapper)((self.extractor)(&row))))
.into_stream();
Ok(it)
}
client: self.client, params: self.params, stmt: self.stmt,
extractor: self.extractor, mapper,
}
pub fn example_query() -> ExampleQueryStmt {
ExampleQueryStmt(cornucopia_async::private::Stmt::new(
"SELECT
} pub async fn one(self) -> Result<T, tokio_postgres::Error>
{
let stmt = self.stmt.prepare(self.client).await?; let row =
self.client.query_one(stmt, &self.params).await?;
Ok((self.mapper)((self.extractor)(&row)))
} pub async fn all(self) -> Result<Vec<T>, tokio_postgres::Error>
{ self.iter().await?.try_collect().await } pub async fn opt(self) ->
Result<Option<T>, tokio_postgres::Error>
{
let stmt = self.stmt.prepare(self.client).await?;
Ok(self.client.query_opt(stmt, &self.params) .await?
.map(|row| (self.mapper)((self.extractor)(&row))))
} pub async fn iter(self,) -> Result<impl futures::Stream<Item = Result<T,
tokio_postgres::Error>> + 'a, tokio_postgres::Error>
{
let stmt = self.stmt.prepare(self.client).await?; let it =
self.client.query_raw(stmt,
cornucopia_async::private::slice_iter(&self.params)) .await?
.map(move |res|
res.map(|row| (self.mapper)((self.extractor)(&row)))) .into_stream();
Ok(it)
}
} pub fn example_query() -> ExampleQueryStmt
{ ExampleQueryStmt(cornucopia_async::private::Stmt::new("SELECT
*
FROM
example_table",
))
}
pub struct ExampleQueryStmt(cornucopia_async::private::Stmt);
impl ExampleQueryStmt {
pub fn bind<'a, C: GenericClient>(
&'a mut self,
client: &'a C,
) -> StringQuery<'a, C, String, 0> {
StringQuery {
client,
params: [],
stmt: &mut self.0,
extractor: |row| row.get(0),
mapper: |it| it.into(),
}
}
}
example_table")) } pub struct
ExampleQueryStmt(cornucopia_async::private::Stmt); impl ExampleQueryStmt
{ pub fn bind<'a, C:
GenericClient,>(&'a mut self, client: &'a C,
) -> StringQuery<'a,C,
String, 0>
{
StringQuery
{
client, params: [], stmt: &mut self.0, extractor:
|row| { row.get(0) }, mapper: |it| { it.into() },
}
}
} } }}