From d7465d03ccc0bc6a31907dd471e73c941ce5be4a Mon Sep 17 00:00:00 2001 From: Javier Cicchelli Date: Wed, 12 Aug 2026 23:57:08 +0200 Subject: [PATCH] Updated the Probe method in the Persistence package with a query deadline. --- Packages/Persistence/README.md | 6 +-- .../Sources/Public/Methods/Probe.swift | 53 +++++++++++++----- .../Cases/Public/Methods/ProbeTests.swift | 40 ++++++++++++++ .../Utils/Fakes/SilentPostgresServer.swift | 54 +++++++++++++++++++ 4 files changed, 138 insertions(+), 15 deletions(-) create mode 100644 Packages/Persistence/Tests/Utils/Fakes/SilentPostgresServer.swift diff --git a/Packages/Persistence/README.md b/Packages/Persistence/README.md index c881d74..a8b7f71 100644 --- a/Packages/Persistence/README.md +++ b/Packages/Persistence/README.md @@ -7,7 +7,7 @@ The [Fluent](https://github.com/hummingbird-project/hummingbird-fluent)-based da | Backend selection | `Driver` (`postgres` or `inMemory`), `Configuration` (the PostgreSQL connection parameters), `TLS` (the connection's TLS posture) | | Service | `Service`, which builds the `Fluent` service configured for the chosen driver | | Migrations | `PrepareDB`, the single registrar declaring every migration, in order | -| Readiness | `Probe`, which reports whether the default database answers a `SELECT 1` | +| Readiness | `Probe`, which reports whether the default database answers a `SELECT 1` within a deadline | | Scaffolding (internal) | `ExampleRecord`, `CreateExampleRecord`, and `ExampleRepository` — the model → migration → repository pattern, to be replaced by the first real domain model | ## Design rules @@ -15,7 +15,7 @@ The [Fluent](https://github.com/hummingbird-project/hummingbird-fluent)-based da - **One default database.** `Service` registers the selected backend as the *default* database, so repositories resolve it with a plain `fluent.db()` and stay agnostic of which driver is in use. - **Migrations are declared once, and append-only.** `PrepareDB` registers every migration in the order it must run; alter the schema by adding a migration, never by editing one that has already run. Registering does not apply them: the in-memory backend migrates on startup, while a shared PostgreSQL database is migrated out of band (the executable's migrate-and-exit mode), so booting instances never race. - **Models never cross a concurrency boundary.** FluentKit models are mutable reference types, so they stay internal to the package and repositories return `Sendable` value-type snapshots (e.g. `Example`) instead. -- **Readiness never throws.** `Probe` runs a `SELECT 1` — the cheapest statement both backends understand, independent of any schema — and maps every failure to `false`, so callers translate it straight into a readiness response. +- **Readiness never throws, and never hangs.** `Probe` runs a `SELECT 1` — the cheapest statement both backends understand, independent of any schema — and maps every failure to `false`, so callers translate it straight into a readiness response. The query is raced against a deadline (2 seconds unless the probe is built with another), so a database that hangs rather than refuses yields a prompt "not ready" instead of a hanging readiness endpoint. - **A single connection for the in-memory store.** The SQLite backend is capped at one connection per event loop, so every query reaches the same in-memory database instead of each pooled connection getting a private one. - **Method structs.** `Service`, `PrepareDB`, and `Probe` hold their lifetime-fixed configuration in `init` and take only per-call inputs in `callAsFunction`. @@ -36,7 +36,7 @@ Sources/ Tests/ ├── Cases/ the test suites, mirroring the Sources/ layout └── Utils/ the NotSQL* fakes backing the probe's non-SQL-database case, the - plaintext-only fake PostgreSQL server, and the suite Tag constants + plaintext-only and silent fake PostgreSQL servers, and the suite Tag constants ``` ## Testing diff --git a/Packages/Persistence/Sources/Public/Methods/Probe.swift b/Packages/Persistence/Sources/Public/Methods/Probe.swift index 78a096d..9282a5f 100644 --- a/Packages/Persistence/Sources/Public/Methods/Probe.swift +++ b/Packages/Persistence/Sources/Public/Methods/Probe.swift @@ -12,14 +12,21 @@ public struct Probe: Sendable { /// The `Fluent` service whose default database is probed. private let fluent: Fluent + /// The longest the probe waits for the database's answer before reporting it as not reachable. + private let timeout: Duration + // MARK: Initializers /// Creates a probe for the default database of the given `Fluent` service. - /// - Parameter fluent: the `Fluent` service whose default database is probed. + /// - Parameters: + /// - fluent: the `Fluent` service whose default database is probed. + /// - timeout: the longest the probe waits for the database's answer before reporting it as not reachable. public init( - fluent: Fluent + fluent: Fluent, + timeout: Duration = .seconds(2) ) { self.fluent = fluent + self.timeout = timeout } // MARK: Methods @@ -28,21 +35,43 @@ public struct Probe: Sendable { /// /// Runs a trivial `SELECT 1` against the default database — the cheapest statement both the PostgreSQL and SQLite backends understand — so /// a readiness check does not depend on any particular schema or model. Any failure (connection refused, authentication error, pool exhausted) is - /// reported as not reachable rather than thrown, so callers can map it straight onto a readiness response. A default database that is not an SQL - /// database is likewise reported as not reachable. - /// - Returns: `true` when the database answers the probe, `false` otherwise. + /// reported as not reachable rather than thrown, and an answer that does not arrive within the timeout is likewise reported as not reachable — so + /// a database that hangs yields a prompt "not ready" instead of a hanging readiness endpoint. A default database that is not an SQL database is + /// also reported as not reachable. + /// - Returns: `true` when the database answers the probe in time, `false` otherwise. public func callAsFunction() async -> Bool { guard let database = fluent.db() as? any SQLDatabase else { return false } - - do { - try await database.raw("SELECT 1").run() - - return true - } catch { - return false + + // The query is raced against the deadline from unstructured tasks whose first answer wins: a structured group + // would await the query child before returning, so a database that hangs mid-query — the very failure the + // deadline exists for — would hang the probe with it. The loser is cancelled and abandoned; a late answer lands + // in a finished stream and is dropped. + let (answers, continuation) = AsyncStream.makeStream(of: Bool.self) + let query = Task { + do { + try await database + .raw("SELECT 1") + .run() + + continuation.yield(true) + } catch { + continuation.yield(false) + } + } + let deadline = Task { + try? await Task.sleep(for: timeout) + + continuation.yield(false) } + var answer = answers.makeAsyncIterator() + let isReachable = await answer.next() ?? false + + query.cancel() + deadline.cancel() + + return isReachable } } diff --git a/Packages/Persistence/Tests/Cases/Public/Methods/ProbeTests.swift b/Packages/Persistence/Tests/Cases/Public/Methods/ProbeTests.swift index 9029fcd..72875c8 100644 --- a/Packages/Persistence/Tests/Cases/Public/Methods/ProbeTests.swift +++ b/Packages/Persistence/Tests/Cases/Public/Methods/ProbeTests.swift @@ -57,6 +57,46 @@ struct ProbeTests { #expect(!isReachable) } + @Test + func `reports a hanging database as unreachable within its timeout`() async throws { + // The silent server accepts the TCP connection and never answers, so the probe's query can only + // ever be resolved by its deadline — without one, it would wait out the driver's own connect + // timeout (10 seconds) instead. + let server = try await SilentPostgresServer.start() + let service = try Service( + driver: .postgres( + .init( + host: "127.0.0.1", + port: server.port, + name: "hanging", + username: "nobody", + password: "nothing", + tls: .off, + maxConnectionsPerEventLoop: 1 + ) + ), + logger: Logger(label: "test") + ) + let fluent = service() + let probe = Probe( + fluent: fluent, + timeout: .milliseconds(100) + ) + + let clock = ContinuousClock() + let start = clock.now + let isReachable = await probe() + let elapsed = clock.now - start + + try await fluent.shutdown() + try await server.stop() + + #expect(!isReachable) + // Well past the 100-millisecond deadline to absorb scheduling noise, yet far below the driver's + // 10-second connect timeout — only the deadline can answer this fast. + #expect(elapsed < .seconds(5)) + } + @Test func `reports a default database that is not an SQL database`() async throws { let fluent = Fluent(logger: Logger(label: "test")) diff --git a/Packages/Persistence/Tests/Utils/Fakes/SilentPostgresServer.swift b/Packages/Persistence/Tests/Utils/Fakes/SilentPostgresServer.swift new file mode 100644 index 0000000..d1498c1 --- /dev/null +++ b/Packages/Persistence/Tests/Utils/Fakes/SilentPostgresServer.swift @@ -0,0 +1,54 @@ +import NIOCore +import NIOPosix + +/// A fake server accepting connections and never answering. +/// +/// A client connecting to it completes the TCP handshake and then waits forever for the first protocol byte — the shape of a database that hangs rather +/// than refuses. This is what the probe's deadline test connects to, proving the probe answers within its timeout instead of hanging alongside the server. +final class SilentPostgresServer { + + // MARK: Properties + + /// The port the server listens on, assigned by the system at bind time. + let port: Int + + /// The listening channel the server accepts connections through. + private let channel: Channel + + // MARK: Initializers + + private init( + channel: Channel, + port: Int + ) { + self.channel = channel + self.port = port + } + + // MARK: Functions + + /// Starts a server on the loopback interface, on a system-assigned port. + /// - Returns: the running server, ready to be connected to at ``port``. + static func start() async throws -> SilentPostgresServer { + let channel = try await ServerBootstrap(group: MultiThreadedEventLoopGroup.singleton) + .bind(host: "127.0.0.1", port: 0) + .get() + + guard let port = channel.localAddress?.port else { + throw ChannelError.unknownLocalAddress + } + + return .init( + channel: channel, + port: port + ) + } + + /// Stops the server, closing its listening channel. + func stop() async throws { + try await channel + .close() + .get() + } + +}