perf(workspace): don't let one database block the others

crates/pgls_cli/src/commands/check.rs

@@ -77,9 +77,9 @@ pub fn check(
         let payload = read_stdin_payload(stdin_path, session.console(), "check")?;
         run_stdin(&mut session, &execution, payload)
     } else {
-        let database_error = check_database_connection(&session)?;
+        let database_errors = check_database_connection(&session)?;
         let mut report: Report = run_files(&mut session, &execution, paths)?;
-        if let Some(error) = database_error {
+        for error in database_errors.into_iter().rev() {
             report.prepend_diagnostic(error);
         }
         for error in configuration_errors.into_iter().rev() {
@@ -94,15 +94,17 @@ pub fn check(
 
 /// A configured database that cannot be reached is an error: the checks that need it would
 /// otherwise be skipped silently.
-fn check_database_connection(session: &CliSession) -> Result<Option<Error>, CliDiagnostic> {
+fn check_database_connection(session: &CliSession) -> Result<Vec<Error>, CliDiagnostic> {
     let result = session
         .workspace()
         .check_database_connection(CheckDatabaseConnectionParams {
             project_key: session.project_key(),
         })?;
     Ok(result
-        .error
-        .map(|message| Error::from(DatabaseUnavailable::new(message))))
+        .errors
+        .into_iter()
+        .map(|message| Error::from(DatabaseUnavailable::new(message)))
+        .collect())
 }
 
 fn resolve_paths(

crates/pgls_cli/tests/assert_check.rs

@@ -696,6 +696,49 @@ fn relative_paths_are_relative_to_the_configuration_file() {
     );
 }
 
+/// `check` verifies the databases of all configuration files at the same time, and reports each
+/// one it can't reach with its address and the folder that uses it.
+#[cfg(target_os = "linux")]
+#[test]
+fn check_reports_every_unreachable_database_at_once() {
+    let unreachable = |port: u16| {
+        format!(
+            r#"{{ "db": {{ "host": "10.255.255.1", "port": {port}, "username": "postgres", "password": "postgres", "connTimeoutSecs": 2 }} }}"#
+        )
+    };
+    let (one, two, three) = (unreachable(5001), unreachable(5002), unreachable(5003));
+    let root = write_tree(&[
+        (
+            "postgres-language-server.jsonc",
+            r#"{ "db": { "disableConnection": true } }"#,
+        ),
+        ("db1/postgres-language-server.jsonc", &one),
+        ("db2/postgres-language-server.jsonc", &two),
+        ("db3/postgres-language-server.jsonc", &three),
+        ("db1/a.sql", "select 1;\n"),
+        ("db2/a.sql", "select 1;\n"),
+        ("db3/a.sql", "select 1;\n"),
+    ]);
+
+    let start = std::time::Instant::now();
+    let (success, output) = check_in(root.path(), &[], &[]);
+    let elapsed = start.elapsed();
+    assert!(!success, "{output}");
+    for (port, folder) in [(5001, "db1"), (5002, "db2"), (5003, "db3")] {
+        let address = format!("postgres@10.255.255.1:{port}/postgres, the database of ");
+        assert!(
+            output.lines().any(|line| line.contains(&address)
+                && line.contains(&root.path().join(folder).display().to_string())),
+            "the database of {folder} is not reported:\n{output}"
+        );
+    }
+    // One connection timeout for the check, not one per database.
+    assert!(
+        elapsed < std::time::Duration::from_secs(5),
+        "the connection checks ran one after the other: {elapsed:?}"
+    );
+}
+
 /// A relative path in the arguments is relative to the working directory, like any other path
 /// typed on the command line, not to the folder of the configuration file.
 #[test]
@@ -885,7 +928,7 @@ fn normalize_durations(input: &str) -> String {
 }
 
 fn normalize_output(status: ExitStatus, stdout: &str, stderr: &str) -> String {
-    let normalized_stdout = normalize_durations(stdout);
+    let normalized_stdout = normalize_test_environment(&normalize_durations(stdout));
     let normalized_stderr = normalize_diagnostics(stderr);
     let status_label = if status.success() {
         "success"
@@ -899,8 +942,28 @@ fn normalize_output(status: ExitStatus, stdout: &str, stderr: &str) -> String {
     )
 }
 
+/// Replaces the address of the database in connection errors, which names the database that
+/// `sqlx::test` creates, and the working directory.
+fn normalize_test_environment(input: &str) -> String {
+    const PREFIX: &str = "Could not connect to ";
+    let mut output = String::new();
+    let mut rest = input;
+    while let Some(start) = rest.find(PREFIX) {
+        output.push_str(&rest[..start + PREFIX.len()]);
+        output.push_str("<database>");
+        rest = &rest[start + PREFIX.len()..];
+        let end = rest.find([',', ' ']).unwrap_or(rest.len());
+        rest = &rest[end..];
+    }
+    output.push_str(rest);
+    match std::env::current_dir() {
+        Ok(cwd) => output.replace(&cwd.display().to_string(), "<cwd>"),
+        Err(_) => output,
+    }
+}
+
 fn normalize_diagnostics(input: &str) -> String {
-    let normalized = normalize_durations(input);
+    let normalized = normalize_test_environment(&normalize_durations(input));
     let mut lines = normalized.lines().peekable();
     let mut diagnostic_sections: Vec<String> = Vec::new();
     let mut other_lines: Vec<String> = Vec::new();

crates/pgls_cli/tests/snapshots/assert_check__check_fails_when_database_login_fails.snap

@@ -9,7 +9,7 @@ Found 1 error.
 stderr:
 database/connection ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  × Database error: password authentication failed for user "postgres"
+  × Could not connect to <database>, the database of <cwd>: Database error: password authentication failed for user "postgres"
 
   i The checks that need the database were skipped. Fix the connection settings, or pass --disable-db or set db.disableConnection to check without a database.
 

crates/pgls_lsp/src/session.rs

@@ -594,7 +594,7 @@ impl Session {
                     project_key: self.project_key(&pgls_path),
                 },
             );
-            if connection.is_ok_and(|connection| connection.error.is_none())
+            if connection.is_ok_and(|connection| connection.errors.is_empty())
                 && self
                     .editor_features_without_database
                     .swap(false, Ordering::Relaxed)

crates/pgls_workspace/src/features/diagnostics.rs

@@ -31,8 +31,8 @@ pub struct CheckDatabaseConnectionParams {
 #[derive(Debug, Default, serde::Serialize, serde::Deserialize)]
 #[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
 pub struct CheckDatabaseConnectionResult {
-    /// Why the configured database cannot be used, if it cannot.
-    pub error: Option<String>,
+    /// Why the configured databases cannot be used, one message per database that can't.
+    pub errors: Vec<String>,
 }
 
 #[derive(Debug, serde::Serialize, serde::Deserialize)]

crates/pgls_workspace/src/settings.rs

@@ -133,11 +133,15 @@ impl WorkspaceSettings {
         self.data.get(key).map(|project| project.settings_for(path))
     }
 
-    /// The root settings and the nested settings of a project.
-    pub fn get_all_settings(&self, key: ProjectKey) -> impl Iterator<Item = &Settings> {
+    /// The root settings and the nested settings of a project, with the folders they apply to.
+    pub fn get_all_settings(&self, key: ProjectKey) -> impl Iterator<Item = (&Path, &Settings)> {
         self.data.get(key).into_iter().flat_map(|project| {
-            std::iter::once(&project.settings)
-                .chain(project.nested.iter().map(|(_, settings)| settings))
+            std::iter::once((project.path.as_path(), &project.settings)).chain(
+                project
+                    .nested
+                    .iter()
+                    .map(|(folder, settings)| (folder.as_path(), settings)),
+            )
         })
     }
 
@@ -769,6 +773,7 @@ impl Default for TypecheckSettings {
 pub const DEFAULT_DATABASE_PASSWORD: &str = "postgres";
 
 /// Database settings for the entire workspace
+#[derive(Clone)]
 pub struct DatabaseSettings {
     pub enable_connection: bool,
     pub connection_string: Option<String>,

crates/pgls_workspace/src/workspace/server.rs

@@ -86,6 +86,10 @@ mod async_helper;
 #[cfg(feature = "db")]
 mod connection_key;
 #[cfg(feature = "db")]
+use crate::settings::DatabaseSettings;
+#[cfg(feature = "db")]
+use connection_key::ConnectionKey;
+#[cfg(feature = "db")]
 mod connection_manager;
 pub(crate) mod document;
 mod expand_star;
@@ -1798,29 +1802,78 @@ impl Workspace for WorkspaceServer {
         &self,
         params: CheckDatabaseConnectionParams,
     ) -> Result<CheckDatabaseConnectionResult, WorkspaceError> {
-        // Every configuration file of the project can have its own database, so check all of them.
-        let settings = self.workspaces();
-        for settings in settings.as_ref().get_all_settings(params.project_key) {
+        // Every configuration file of the project can have its own database, so check each of
+        // them once, with the folders that use it.
+        let mut connections: Vec<(DatabaseSettings, Vec<PathBuf>)> = Vec::new();
+        let mut errors = Vec::new();
+        for (folder, settings) in self
+            .workspaces()
+            .as_ref()
+            .get_all_settings(params.project_key)
+        {
+            // An invalid connection string disables the connection, so say why.
             if let Some(error) = &settings.db.connection_string_error {
-                return Ok(CheckDatabaseConnectionResult {
-                    error: Some(error.clone()),
-                });
+                if !errors.contains(error) {
+                    errors.push(error.clone());
+                }
+                continue;
+            }
+            if !settings.db.enable_connection {
+                continue;
+            }
+            let existing = connections.iter_mut().find(|(db, _)| {
+                ConnectionKey::from(db) == ConnectionKey::from(&settings.db)
+                    && db.connection_string == settings.db.connection_string
+                    && db.password == settings.db.password
+                    && db.conn_timeout_secs == settings.db.conn_timeout_secs
+            });
+            match existing {
+                Some((_, folders)) => folders.push(folder.to_path_buf()),
+                None => connections.push((settings.db.clone(), vec![folder.to_path_buf()])),
             }
+        }
+
+        // Concurrently, so that unreachable databases time out together rather than one after
+        // the other.
+        let results: Vec<_> = std::thread::scope(|scope| {
+            let checks: Vec<_> = connections
+                .iter()
+                .map(|(db, _)| {
+                    scope.spawn(|| {
+                        self.connection
+                            .try_with_pool(db, |pool| self.snapshot.load(pool).map(|_| ()))
+                    })
+                })
+                .collect();
+            checks
+                .into_iter()
+                .map(|check| check.join().expect("a connection check panicked"))
+                .collect()
+        });
 
-            let result = self
-                .connection
-                .try_with_pool(&settings.db, |pool| self.snapshot.load(pool).map(|_| ()));
+        for ((db, folders), result) in connections.iter().zip(results) {
             match result {
                 None | Some(Ok(())) => {}
                 Some(Err(err @ WorkspaceError::DatabaseConnectionError(_))) => {
-                    return Ok(CheckDatabaseConnectionResult {
-                        error: Some(err.to_string()),
-                    });
+                    let folders: Vec<_> = folders
+                        .iter()
+                        .filter(|folder| !folder.as_os_str().is_empty())
+                        .map(|folder| folder.display().to_string())
+                        .collect();
+                    let used_by = if folders.is_empty() {
+                        String::new()
+                    } else {
+                        format!(", the database of {}", folders.join(", "))
+                    };
+                    errors.push(format!(
+                        "Could not connect to {}@{}:{}/{}{used_by}: {err}",
+                        db.username, db.host, db.port, db.database
+                    ));
                 }
                 Some(Err(err)) => return Err(err),
             }
         }
-        Ok(CheckDatabaseConnectionResult::default())
+        Ok(CheckDatabaseConnectionResult { errors })
     }
 
     #[cfg(not(feature = "db"))]

crates/pgls_workspace/src/workspace/server/snapshot_manager.rs

@@ -1,5 +1,8 @@
 use std::sync::{Arc, RwLock};
 
+#[cfg(feature = "db")]
+use std::sync::Mutex;
+
 use pgls_catalog::Snapshot;
 
 use crate::WorkspaceError;
@@ -12,6 +15,10 @@ use std::collections::HashMap;
 #[cfg(feature = "db")]
 use super::{async_helper::run_async, connection_key::ConnectionKey};
 
+/// The snapshot of a connection, once it loaded.
+#[cfg(feature = "db")]
+type SnapshotSlot = Arc<Mutex<Option<Arc<Snapshot>>>>;
+
 /// Manages database snapshot storage and retrieval.
 ///
 /// In db mode: supports loading from database connections and/or JSON.
@@ -26,9 +33,10 @@ use super::{async_helper::run_async, connection_key::ConnectionKey};
 /// - `load()` - Load schema from database connection
 /// - `clear_all_connections()` - Clear the schemas of all connections
 pub struct SnapshotManager {
-    /// Connection-based snapshots (db mode only)
+    /// Connection-based snapshots (db mode only). Each connection has its own lock, held while
+    /// its snapshot loads, so that a slow or unreachable database doesn't block the others.
     #[cfg(feature = "db")]
-    db_schemas: RwLock<HashMap<ConnectionKey, Arc<Snapshot>>>,
+    db_schemas: RwLock<HashMap<ConnectionKey, SnapshotSlot>>,
 
     /// JSON-loaded schema (available in both modes)
     schema: RwLock<Option<Arc<Snapshot>>>,
@@ -77,29 +85,26 @@ impl SnapshotManager {
     pub fn load(&self, pool: &PgPool) -> Result<Arc<Snapshot>, WorkspaceError> {
         let key: ConnectionKey = pool.into();
 
-        // Try read lock first for cache hit
-        if let Ok(schemas) = self.db_schemas.read()
-            && let Some(cache) = schemas.get(&key)
-        {
-            return Ok(Arc::clone(cache));
-        }
-
-        // Cache miss - need write lock to insert
-        let mut schemas = self.db_schemas.write().unwrap();
+        // The map is only locked to find the connection's slot, never during network I/O.
+        let existing = self.db_schemas.read().unwrap().get(&key).cloned();
+        let slot = match existing {
+            Some(slot) => slot,
+            None => Arc::clone(self.db_schemas.write().unwrap().entry(key).or_default()),
+        };
 
-        // Double-check after acquiring write lock
-        if let Some(cache) = schemas.get(&key) {
+        // Callers for the same connection wait for one load instead of starting their own.
+        let mut snapshot = slot.lock().unwrap();
+        if let Some(cache) = snapshot.as_ref() {
             return Ok(Arc::clone(cache));
         }
 
-        // Load the snapshot from the database
+        // Load the snapshot from the database. A failure leaves the slot empty for a retry.
         let pool_clone = pool.clone();
-        let snapshot = Arc::new(run_async(
+        let loaded = Arc::new(run_async(
             async move { Snapshot::load(&pool_clone).await },
         )??);
-
-        schemas.insert(key, snapshot.clone());
-        Ok(snapshot)
+        *snapshot = Some(Arc::clone(&loaded));
+        Ok(loaded)
     }
 
     /// Clear all connection-based snapshots.