-
Notifications
You must be signed in to change notification settings - Fork 122
fix: delay on startup when connecting unreachable local postgres db #1246
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
Changes from all commits
b59f308
6c596e5
48d9c66
1d2e130
453cc9b
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,27 +1,68 @@ | ||||||||||
| use common::config::DbConfig; | ||||||||||
| use common::errors::MegaError; | ||||||||||
| use sea_orm::{ConnectOptions, Database, DatabaseConnection}; | ||||||||||
| use std::{path::Path, time::Duration}; | ||||||||||
| use std::{ | ||||||||||
| net::{SocketAddr, TcpStream}, | ||||||||||
| path::Path, | ||||||||||
| time::Duration, | ||||||||||
| }; | ||||||||||
| use tracing::log; | ||||||||||
| use url::Url; | ||||||||||
|
|
||||||||||
| use crate::migration::apply_migrations; | ||||||||||
|
|
||||||||||
| use crate::utils::id_generator; | ||||||||||
|
|
||||||||||
| /// Create a database connection. | ||||||||||
| /// When postgres is set but not available, it will fall back to sqlite automatically. | ||||||||||
| /// Create a database connection with failover logic. | ||||||||||
| /// | ||||||||||
| /// This function attempts to connect to a database based on the provided configuration: | ||||||||||
| /// - If PostgreSQL is specified but unavailable, it automatically falls back to SQLite | ||||||||||
| /// - For local PostgreSQL connections, it first checks port reachability to avoid long timeouts | ||||||||||
| /// | ||||||||||
| /// The failover logic works as follows: | ||||||||||
| /// 1. For PostgreSQL connections: | ||||||||||
| /// - If the host is local (localhost, 127.0.0.1, etc.), performs a quick port check (100ms timeout) | ||||||||||
| /// - If port is unreachable, immediately falls back to SQLite without waiting for a full connection timeout | ||||||||||
| /// - If port is reachable but connection fails, logs the error and falls back to SQLite | ||||||||||
| /// 2. For non-local PostgreSQL, attempts connection with normal timeouts (3 seconds) | ||||||||||
| /// - On failure, logs the error and falls back to SQLite | ||||||||||
| /// 3. For SQLite connections, connects directly without fallback | ||||||||||
| /// | ||||||||||
| /// After successful connection, applies any pending database migrations. | ||||||||||
| /// | ||||||||||
| /// This optimization helps avoid long waits when local PostgreSQL isn't running. | ||||||||||
| pub async fn database_connection(db_config: &DbConfig) -> DatabaseConnection { | ||||||||||
| id_generator::set_up_options().unwrap(); | ||||||||||
|
|
||||||||||
| let conn = if db_config.db_type == "postgres" { | ||||||||||
| match postgres_connection(db_config).await { | ||||||||||
| Ok(conn) => conn, | ||||||||||
| Err(e) => { | ||||||||||
| log::error!("Failed to connect to postgres: {e}"); | ||||||||||
| log::info!("Falling back to sqlite"); | ||||||||||
| if should_check_port_first(&db_config.db_url) { | ||||||||||
| if !is_port_reachable(&db_config.db_url) { | ||||||||||
| log::info!("Local postgres port not reachable, falling back to sqlite"); | ||||||||||
| sqlite_connection(db_config) | ||||||||||
| .await | ||||||||||
| .expect("Cannot connect to any database") | ||||||||||
| } else { | ||||||||||
| match postgres_connection(db_config).await { | ||||||||||
| Ok(conn) => conn, | ||||||||||
| Err(e) => { | ||||||||||
| log::error!("Failed to connect to postgres: {e}"); | ||||||||||
| log::info!("Falling back to sqlite"); | ||||||||||
| sqlite_connection(db_config) | ||||||||||
| .await | ||||||||||
| .expect("Cannot connect to any database") | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } else { | ||||||||||
| match postgres_connection(db_config).await { | ||||||||||
| Ok(conn) => conn, | ||||||||||
| Err(e) => { | ||||||||||
| log::error!("Failed to connect to postgres: {e}"); | ||||||||||
| log::info!("Falling back to sqlite"); | ||||||||||
| sqlite_connection(db_config) | ||||||||||
| .await | ||||||||||
| .expect("Cannot connect to any database") | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } else { | ||||||||||
|
|
@@ -34,6 +75,29 @@ pub async fn database_connection(db_config: &DbConfig) -> DatabaseConnection { | |||||||||
| conn | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn should_check_port_first(db_url: &str) -> bool { | ||||||||||
| if let Ok(url) = Url::parse(db_url) { | ||||||||||
| if let Some(host) = url.host_str() { | ||||||||||
| return host == "localhost" | ||||||||||
| || host == "127.0.0.1" | ||||||||||
| || host == "::1" | ||||||||||
| || host == "0.0.0.0"; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| false | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn is_port_reachable(db_url: &str) -> bool { | ||||||||||
| if let Ok(url) = Url::parse(db_url) { | ||||||||||
| if let (Some(host), Some(port)) = (url.host_str(), url.port()) { | ||||||||||
| if let Ok(addr) = format!("{host}:{port}").parse::<SocketAddr>() { | ||||||||||
| return TcpStream::connect_timeout(&addr, Duration::from_millis(100)).is_ok(); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| false | ||||||||||
| } | ||||||||||
|
|
||||||||||
| async fn postgres_connection(db_config: &DbConfig) -> Result<DatabaseConnection, MegaError> { | ||||||||||
| let db_url = db_config.db_url.to_owned(); | ||||||||||
| log::info!("Connecting to database: {db_url}"); | ||||||||||
|
|
@@ -61,11 +125,59 @@ fn setup_option(db_url: impl Into<String>) -> ConnectOptions { | |||||||||
| let mut opt = ConnectOptions::new(db_url); | ||||||||||
| opt.max_connections(5) | ||||||||||
| .min_connections(1) | ||||||||||
| .acquire_timeout(Duration::from_secs(3)) | ||||||||||
| .connect_timeout(Duration::from_secs(3)) | ||||||||||
| .acquire_timeout(Duration::from_secs(1)) | ||||||||||
|
||||||||||
| .acquire_timeout(Duration::from_secs(1)) | |
| .acquire_timeout(Duration::from_secs(2)) |
Copilot
AI
Jul 19, 2025
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.
Reducing connect_timeout from 3 seconds to 1 second may cause legitimate connections to fail, especially in containerized or network-constrained environments. This aggressive timeout could lead to unnecessary fallbacks to SQLite.
| .connect_timeout(Duration::from_secs(1)) | |
| .connect_timeout(Duration::from_secs(3)) |
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.
HELL NO
Copilot
AI
Jul 22, 2025
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.
The connect_timeout has been reduced from 3 seconds to 1 second. While this helps with fast failure detection, it may be too aggressive for legitimate connections that take longer to establish, especially in containerized environments or under load.
| .connect_timeout(Duration::from_secs(1)) | |
| .connect_timeout(Duration::from_secs(3)) |
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.
Reducing acquire_timeout from 3 seconds to 1 second may cause connection failures under high load or slow network conditions. Consider keeping the original 3-second timeout or making this configurable.