diff --git a/CHANGELOG.md b/CHANGELOG.md index 75927e9357..3a738e308d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- PostgreSQL, Redshift and CockroachDB keep one connection per database in the connections strip, and send TCP keepalives so idle ones stay open. + ### Fixed +- Switching between two databases of a PostgreSQL connection reconnecting each time and dropping the open transaction and temp tables. +- A tab on a database the connection had switched away from running on a shared connection that closed after 10 minutes idle. +- Import and Copy To committing a transaction left open on the target connection. +- Health check reconnecting a PostgreSQL session that was sitting in a failed transaction. - Each switch between connections in the connections strip reloading that connection's schema. - Opening a connection fetching every column of its schema twice. - Unsaved grid edits not kept with their tab after jumping to a tab of a background connection. diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQConnectionLoss.swift b/Plugins/PostgreSQLDriverPlugin/LibPQConnectionLoss.swift index dc6963e639..293af0eab8 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQConnectionLoss.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQConnectionLoss.swift @@ -51,6 +51,38 @@ enum LibPQServerMessage { } } +/// How a health check asks whether a session is still there without disturbing what the user has +/// open in it. +internal enum LibPQSessionCheck { + static let statement = "SELECT 1" + + /// Never inside a transaction block. In an aborted one the server refuses every statement with + /// `25P02` while the session is fine, and in an open one a statement can take a repeatable-read + /// snapshot early and resets `idle_in_transaction_session_timeout`. Reading the socket, which + /// the caller does first, still sees a server that closed the session. + static func sendsStatement(in state: LibPQTransactionState) -> Bool { + switch state { + case .inTransaction, .inError: + return false + case .idle, .active, .unknown: + return true + } + } + + /// The SQLSTATE of a server that refused the check statement, or nil when the failure is not + /// the server's answer. libpq's own failures carry no SQLSTATE and a lost session arrives as + /// `LibPQConnectionLostError`. A FATAL carries a SQLSTATE too, so the caller also requires + /// `PQstatus` to read `CONNECTION_OK` afterwards: `PQexec` reads past a FATAL to the closed + /// socket and turns it `CONNECTION_BAD`. + static func refusalState(of error: Error) -> String? { + guard let error = error as? LibPQPluginError, + let sqlState = error.sqlState, + !sqlState.isEmpty + else { return nil } + return sqlState + } +} + enum LibPQConnectionLoss: Sendable, Equatable { case beforeSending(transactionMayBeOpen: Bool) case afterSending diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQConnectionString.swift b/Plugins/PostgreSQLDriverPlugin/LibPQConnectionString.swift index 85bc501780..9400ee32ba 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQConnectionString.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQConnectionString.swift @@ -74,6 +74,18 @@ internal enum LibPQConnectionString { static let sessionApplicationName = "TablePro" static let metadataApplicationName = "TablePro Metadata" + /// Left to the kernel, macOS sends the first keepalive after 7,200 s of idle, while an AWS + /// Network Load Balancer drops an idle flow after 350 s and an Azure Load Balancer silently after + /// 4 minutes. These keep a parked session's flow open and let the kernel declare a dead peer + /// about 90 s after the last traffic. `tcp_user_timeout` is left out: macOS has no + /// `TCP_USER_TIMEOUT`, so libpq accepts it and changes nothing. + static let keepaliveParameters: [(String, String)] = [ + ("keepalives", "1"), + ("keepalives_idle", "60"), + ("keepalives_interval", "10"), + ("keepalives_count", "3") + ] + static func build( host: String, port: Int, @@ -100,6 +112,7 @@ internal enum LibPQConnectionString { if let connectTimeoutSeconds { parameters.append(("connect_timeout", String(max(connectTimeoutSeconds, 1)))) } + parameters.append(contentsOf: keepaliveParameters) parameters.append(("sslmode", LibPQSSLMapping.sslmode(for: sslConfig.mode))) if sslConfig.verifiesCertificate, !sslConfig.caCertificatePath.isEmpty { diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift b/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift index 0da58eda6f..66f2d19999 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQDriverCore.swift @@ -164,7 +164,7 @@ final class LibPQDriverCore: @unchecked Sendable { guard let pqConn = libpqConnection else { throw LibPQPluginError.notConnected } - _ = try await pqConn.executeQuery("SELECT 1") + try await pqConn.ping() } // MARK: - Query Execution diff --git a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift index d0b478742f..5beb75ad45 100644 --- a/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift +++ b/Plugins/PostgreSQLDriverPlugin/LibPQPluginConnection.swift @@ -658,6 +658,30 @@ final class LibPQPluginConnection: @unchecked Sendable { return Self.transactionState(PQtransactionStatus(conn)) } + /// libpq has no round trip that sends no statement, so the socket is read first: that sees a + /// server that closed the session, and inside a transaction block it is the whole check + /// (`LibPQSessionCheck.sendsStatement`). A statement the server refuses still proves the + /// backend answered, so it does not fail the ping. + func ping() async throws { + try await pluginDispatchAsync(on: queue) { [self] in + guard !isShuttingDown, let conn = connectionHandle else { throw LibPQPluginError.notConnected } + if let ended = sessionEndedBeforeSending(conn) { throw ended } + guard LibPQSessionCheck.sendsStatement(in: transactionStateOnQueue()) else { return } + do { + if let deadline = activeConnectDeadline { + _ = try executeConnectQuerySync(LibPQSessionCheck.statement, deadline: deadline) + } else { + _ = try executeQuerySync(LibPQSessionCheck.statement) + } + } catch { + guard PQstatus(conn) == CONNECTION_OK, + let sqlState = LibPQSessionCheck.refusalState(of: error) + else { throw error } + Self.logger.info("Ping refused with SQLSTATE \(sqlState, privacy: .public) on a live session") + } + } + } + func boundedQuery(_ query: String, rowCap: Int) async throws -> LibPQPluginQueryResult { let queryToRun = String(query) let cap = max(rowCap, 1) diff --git a/TablePro/Core/Concurrency/SessionDriverGate.swift b/TablePro/Core/Concurrency/SessionDriverGate.swift index 35aec640ab..faa9fe5421 100644 --- a/TablePro/Core/Concurrency/SessionDriverGate.swift +++ b/TablePro/Core/Concurrency/SessionDriverGate.swift @@ -5,12 +5,16 @@ import Foundation -/// Serialises access to a connection's single shared driver. +/// Serialises access to a connection's shared session drivers. /// /// The driver carries one mutable position (its current database and schema), so an /// operation has to move it before it runs. Without ordering, two windows interleave /// their moves and each runs against the other's database. /// +/// A connection that keeps one driver per database (see `SessionLanes`) takes one turn per +/// database: each of those drivers sits on its own database for good, so work on two databases +/// never has to wait for the other, while two operations on one database still take turns. +/// /// The body runs inline in the caller's own task rather than in a detached one, so /// cancellation still reaches the work. /// @@ -19,69 +23,91 @@ import Foundation /// release must not free or hand off a turn a later session has taken since. @MainActor final class SessionDriverGate { + /// One turn per connection, or per database of a connection that keeps a driver per database. + struct Key: Hashable { + let connectionId: UUID + let database: String? + } + private struct Waiter { let ticket: UUID let continuation: CheckedContinuation } - private var owners: [UUID: UUID] = [:] - private var waiters: [UUID: [Waiter]] = [:] + private var owners: [Key: UUID] = [:] + private var waiters: [Key: [Waiter]] = [:] func withExclusiveAccess( _ connectionId: UUID, _ body: () async throws -> T ) async throws -> T { - let ticket = try await acquire(connectionId) - defer { release(connectionId, ticket: ticket) } + try await withExclusiveAccess(Key(connectionId: connectionId, database: nil), body) + } + + func withExclusiveAccess( + _ key: Key, + _ body: () async throws -> T + ) async throws -> T { + let ticket = try await acquire(key) + defer { release(key, ticket: ticket) } return try await body() } + /// Whether a turn is running on `key`, which for a connection's own driver is proof that it is + /// answering without asking it again. + func isHeld(_ key: Key) -> Bool { + owners[key] != nil + } + #if DEBUG /// How many callers are queued behind the holder, so a test can wait for one to reach the /// gate instead of guessing how many scheduler turns that takes. internal func waiterCount(for connectionId: UUID) -> Int { - waiters[connectionId]?.count ?? 0 + waiters.filter { $0.key.connectionId == connectionId }.values.reduce(0) { $0 + $1.count } } #endif - /// Releases a connection that is going away, failing everyone still queued for it. + /// Releases a connection that is going away, failing everyone still queued for any of its turns. func drain(connectionId: UUID) { - owners.removeValue(forKey: connectionId) - let pending = waiters.removeValue(forKey: connectionId) ?? [] - for waiter in pending { - waiter.continuation.resume(throwing: CancellationError()) + let keys = Set(owners.keys).union(waiters.keys).filter { $0.connectionId == connectionId } + for key in keys { + owners.removeValue(forKey: key) + let pending = waiters.removeValue(forKey: key) ?? [] + for waiter in pending { + waiter.continuation.resume(throwing: CancellationError()) + } } } - private func acquire(_ connectionId: UUID) async throws -> UUID { + private func acquire(_ key: Key) async throws -> UUID { let ticket = UUID() - guard owners[connectionId] != nil else { - owners[connectionId] = ticket + guard owners[key] != nil else { + owners[key] = ticket return ticket } try await withTaskCancellationHandler( - operation: { try await enqueue(ticket: ticket, connectionId: connectionId) }, + operation: { try await enqueue(ticket: ticket, key: key) }, onCancel: { [weak self] in Task { @MainActor in - self?.failWaiter(ticket: ticket, connectionId: connectionId) + self?.failWaiter(ticket: ticket, key: key) } } ) /// A hand-off resumes this caller before it runs, so a drain can land in between, and the /// turn it was handed ended with that drain. - guard owners[connectionId] == ticket else { + guard owners[key] == ticket else { throw CancellationError() } return ticket } - private func enqueue(ticket: UUID, connectionId: UUID) async throws { + private func enqueue(ticket: UUID, key: Key) async throws { try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in guard !Task.isCancelled else { continuation.resume(throwing: CancellationError()) return } - waiters[connectionId, default: []].append( + waiters[key, default: []].append( Waiter(ticket: ticket, continuation: continuation) ) } @@ -89,27 +115,27 @@ final class SessionDriverGate { /// Removes the ticket before resuming it, so a cancellation racing a hand-off /// can only ever find one of them. - private func failWaiter(ticket: UUID, connectionId: UUID) { - guard var pending = waiters[connectionId], + private func failWaiter(ticket: UUID, key: Key) { + guard var pending = waiters[key], let index = pending.firstIndex(where: { $0.ticket == ticket }) else { return } let waiter = pending.remove(at: index) - waiters[connectionId] = pending.isEmpty ? nil : pending + waiters[key] = pending.isEmpty ? nil : pending waiter.continuation.resume(throwing: CancellationError()) } - private func release(_ connectionId: UUID, ticket: UUID) { - guard owners[connectionId] == ticket else { return } - guard var pending = waiters[connectionId], !pending.isEmpty else { - owners.removeValue(forKey: connectionId) - waiters.removeValue(forKey: connectionId) + private func release(_ key: Key, ticket: UUID) { + guard owners[key] == ticket else { return } + guard var pending = waiters[key], !pending.isEmpty else { + owners.removeValue(forKey: key) + waiters.removeValue(forKey: key) return } let next = pending.removeFirst() - waiters[connectionId] = pending.isEmpty ? nil : pending - owners[connectionId] = next.ticket + waiters[key] = pending.isEmpty ? nil : pending + owners[key] = next.ticket next.continuation.resume() } } diff --git a/TablePro/Core/Database/DatabaseManager+Health.swift b/TablePro/Core/Database/DatabaseManager+Health.swift index fdd22a1d4b..a32e66c65f 100644 --- a/TablePro/Core/Database/DatabaseManager+Health.swift +++ b/TablePro/Core/Database/DatabaseManager+Health.swift @@ -58,6 +58,7 @@ extension DatabaseManager { Self.logger.debug("Ping skipped — no active driver for \(connectionId)") return false } + await self.notePinged(mainDriver, for: connectionId) do { try await mainDriver.ping() await self.markSessionVerified(connectionId) @@ -69,7 +70,10 @@ extension DatabaseManager { }, reconnectHandler: { [weak self] in guard let self else { return .abort } - return await self.performHealthMonitorReconnect(connectionId: connectionId) + return await self.performHealthMonitorReconnect( + connectionId: connectionId, + failedDriver: await self.pingedDriver(for: connectionId) + ) }, onStateChanged: { [weak self] id, state in guard let self else { return } @@ -112,19 +116,33 @@ extension DatabaseManager { /// is not a teardown, and clearing the cache here leaves the sidebar and autocomplete empty /// with nothing scheduled to refill them. Success publishes `databaseDidConnect` so the same /// listeners that reload after a first connect or a manual reconnect run here too. - internal func performHealthMonitorReconnect(connectionId: UUID) async -> ConnectionHealthMonitor.ReconnectOutcome { + internal func performHealthMonitorReconnect( + connectionId: UUID, + failedDriver: ObjectIdentifier? = nil + ) async -> ConnectionHealthMonitor.ReconnectOutcome { guard let session = activeSessions[connectionId] else { return .abort } + /// The check that failed was made on one driver, and a database switch may have parked it and + /// put another one in its place since. Reconnecting now would disconnect the connection the + /// user just moved onto, with its transaction; the parked one is checked again before use. + if let failedDriver, let current = session.driver, ObjectIdentifier(current) != failedDriver { + return .success + } /// The driver this attempt is replacing. Every give-up below is fenced on it, because a /// reconnect blocked inside a C call cannot be cancelled and completes late: without the /// fence, a losing attempt would report a connection unreachable that a later one restored. let attemptedDriver = session.driver - await SchemaService.shared.prepareForReload(connectionId: connectionId) - await DatabaseTreeMetadataService.shared.handleReconnect(connectionId: connectionId) /// A connection that stopped answering has most likely taken its pooled connections with it, /// and a rebuilt tunnel moves every one of them to a new port, so pooled work waits for the - /// replacement rather than dialing what is being torn down. + /// replacement rather than dialing what is being torn down. Begun before anything suspends, + /// so a database switch cannot promote another connection in the meantime. MetadataConnectionPool.shared.beginTransportReplacement(connectionId: connectionId) defer { MetadataConnectionPool.shared.endTransportReplacement(connectionId: connectionId) } + await SchemaService.shared.prepareForReload(connectionId: connectionId) + await DatabaseTreeMetadataService.shared.handleReconnect(connectionId: connectionId) + /// Asked again after the suspensions above: a switch that landed before the replacement + /// began installed another database's connection, which must not be disconnected for a + /// failure it never had. + guard activeSessions[connectionId]?.driver === attemptedDriver else { return .success } do { guard let result = try await trackOperation(sessionId: connectionId, operation: { @@ -255,6 +273,9 @@ extension DatabaseManager { // Rebuild the tunnel if needed; otherwise reuse effective connection let connectionForDriver: DatabaseConnection if session.connection.activeTunnelKind != nil { + /// Rebuilding the tunnel moves it to a new local port, which strands every other + /// database's connection on the old one. + await sessionLanes.closeAllNotingLostTransactions(for: session.connection.id) connectionForDriver = try await buildEffectiveConnection( for: session.connection, deadline: deadline @@ -377,6 +398,9 @@ extension DatabaseManager { let replacesPooledTransport = session.connection.activeTunnelKind != nil || session.liveness != .live if replacesPooledTransport { MetadataConnectionPool.shared.beginTransportReplacement(connectionId: sessionId) + /// The other databases' connections were dialed through the same transport, so they go + /// with it and reopen on their next use. + await sessionLanes.closeAllNotingLostTransactions(for: sessionId) } defer { if replacesPooledTransport { diff --git a/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift b/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift index 99211b7e29..0f7abab750 100644 --- a/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift +++ b/TablePro/Core/Database/DatabaseManager+ScopedDriver.swift @@ -66,6 +66,15 @@ extension DatabaseManager { else { return .sessionDriver } + /// A database the user has browsed keeps its own session connection, so its tabs carry on in + /// the transaction, temp tables and settings they left there. + /// A database whose connection ended with a transaction still open takes the session path + /// once more, where that loss is reported before anything runs on a replacement. + if usesDatabaseLanes(session), + sessionLanes.parkedDriver(for: scope.connectionId, database: scope.database) != nil + || sessionLanes.hasTransactionLoss(database: scope.database, for: scope.connectionId) { + return .sessionDriver + } guard canPool(session) else { return .unavailable( String( @@ -289,7 +298,7 @@ extension DatabaseManager { /// serve the wrong database entirely. And one whose database lives inside the driver /// instance, rather than on a server it reconnects to, hands the pool a different database /// altogether: `supportsConnectionPooling` is how those opt out. - private func canPool(_ session: ConnectionSession) -> Bool { + internal func canPool(_ session: ConnectionSession) -> Bool { guard session.connection.type.supportsConnectionPooling else { return false } let actions = PluginMetadataRegistry.shared.snapshot( for: session.connection.type @@ -300,6 +309,15 @@ extension DatabaseManager { } } + /// Whether browsing another database keeps one connection per database instead of reconnecting. + /// It takes an engine that has to reconnect to change database, and a server that accepts a + /// second connection to the same definition: the test pooling already answers. + internal func usesDatabaseLanes(_ session: ConnectionSession) -> Bool { + guard PluginMetadataRegistry.shared.snapshot(for: session.connection.type)? + .capabilities.requiresReconnectForDatabaseSwitch == true else { return false } + return canPool(session) + } + /// Whether the connection is running work that must not be interrupted, whatever its age. internal func holdsProtectedWrite(_ connectionId: UUID) -> Bool { (runningDrivers[connectionId] ?? [:]).values.contains { $0.policy == .protectedWrite } @@ -309,7 +327,7 @@ extension DatabaseManager { scope: DatabaseScope, _ body: @Sendable @escaping (DatabaseDriver) async throws -> T ) async throws -> T { - try await withSessionDriverTurn(connectionId: scope.connectionId) { driver in + try await withSessionDriverTurn(scope: scope) { driver in try await pin(driver, to: scope) return try await body(driver) } @@ -322,7 +340,7 @@ extension DatabaseManager { scope: DatabaseScope, _ body: @Sendable @escaping (DatabaseDriver) async throws -> T ) async throws -> TableReadTurn { - try await withSessionDriverTurn(connectionId: scope.connectionId) { driver in + try await withSessionDriverTurn(scope: scope) { driver in let route = executionRoute(for: scope) guard route == .sessionDriver else { return .moved(route) } try await pin(driver, to: scope) @@ -331,32 +349,129 @@ extension DatabaseManager { } private func withSessionDriverTurn( - connectionId: UUID, + scope: DatabaseScope, _ turn: (DatabaseDriver) async throws -> R ) async throws -> R { - /// Outside the gate on purpose. A verification that has to reconnect runs the whole - /// reconnect, which restores the schema and the database on the new driver, and doing that - /// while holding the gate would deadlock the very thing waiting to be pinned. - await verifyBeforeUse(connectionId) + let connectionId = scope.connectionId + let lane = activeSessions[connectionId].flatMap { laneDatabase(for: scope, in: $0) } + let startsOnBrowsed = lane == nil || lane == activeSessions[connectionId]?.resolvedBrowseDatabase + if startsOnBrowsed { + /// Outside the gate on purpose. A verification that has to reconnect runs the whole + /// reconnect, which restores the schema and the database on the new driver, and doing that + /// while holding the gate would deadlock the very thing waiting to be pinned. + await verifyBeforeUse(connectionId) + } /// A check that failed and could not recover left the driver installed and disconnected, /// so the presence of a driver below is not enough. Refusing here is the point of checking /// at all: without it the user's own work runs on a handle the app already knows is dead. guard isUsable(connectionId) else { throw DatabaseError.notConnected } - return try await sessionDriverGate.withExclusiveAccess(connectionId) { + return try await sessionDriverGate.withExclusiveAccess( + SessionDriverGate.Key(connectionId: connectionId, database: lane) + ) { try await trackOperation(sessionId: connectionId) { try Task.checkCancellation() /// Asked again once the lease has its turn, because a database switch that held the - /// gate can have left the driver the same way while this lease waited. - guard isUsable(connectionId), let driver = driver(for: connectionId) else { + /// gate can have left the driver the same way while this lease waited, or moved the + /// database this turn belongs to from the browsed connection to a parked one. + /// A database whose own connection is gone, its entry closed, falls back to the browsed + /// driver only to be turned away: a table read re-routes to the pool and anything else + /// is refused by `pin` before it runs a statement there. + guard isUsable(connectionId), + let driver = sessionDriver(for: connectionId, laneDatabase: lane) + ?? activeSessions[connectionId]?.driver + else { throw DatabaseError.notConnected } - return try await turn(driver) + guard let lane else { return try await turn(driver) } + let laneDriver = sessionLanes.isParked(driver, for: connectionId) + ? try await verifiedParkedDriver(driver, database: lane, for: connectionId) + : driver + if sessionLanes.takeTransactionLoss(database: lane, for: connectionId) { + throw DatabaseError.connectionFailed(Self.parkedTransactionLostMessage(database: lane)) + } + return try await turn(laneDriver) + } + } + } + + /// The turn that guards the browsed connection: per database for a connection that keeps one + /// connection per database, per connection otherwise. + internal func browsedGateKey(for connectionId: UUID) -> SessionDriverGate.Key { + guard let session = activeSessions[connectionId], usesDatabaseLanes(session) else { + return SessionDriverGate.Key(connectionId: connectionId, database: nil) + } + return SessionDriverGate.Key(connectionId: connectionId, database: session.resolvedBrowseDatabase) + } + + /// The database whose session connection a scope runs on, for a connection that keeps one per + /// database; nil for every other connection, which has the one shared driver. + private func laneDatabase(for scope: DatabaseScope, in session: ConnectionSession) -> String? { + guard usesDatabaseLanes(session) else { return nil } + return scope.isServerScoped ? session.resolvedBrowseDatabase : scope.database + } + + /// The driver holding `laneDatabase` right now: the browsed one, or the one parked for it. + private func sessionDriver(for connectionId: UUID, laneDatabase: String?) -> DatabaseDriver? { + guard let session = activeSessions[connectionId] else { return nil } + guard let laneDatabase, laneDatabase != session.resolvedBrowseDatabase else { + return session.driver + } + return sessionLanes.parkedDriver(for: connectionId, database: laneDatabase) + } + + /// A parked connection nobody watched may have died while it sat idle. One that no longer + /// answers is replaced before the work runs, and if it was holding a transaction the work is + /// refused once, because running it on the new connection would carry on as if that transaction + /// had not just been rolled back by the server. + private func verifiedParkedDriver( + _ driver: DatabaseDriver, + database: String, + for connectionId: UUID + ) async throws -> DatabaseDriver { + if sessionLanes.isFresh(driver) { return driver } + /// Read before the ping: a ping that finds the socket closed leaves libpq reporting no + /// transaction state at all, which would read as nothing to lose. + let held = await driver.heldSessionTransactionState() + sessionLanes.beginVerifying(driver) + defer { sessionLanes.endVerifying(driver) } + do { + try await driver.ping() + sessionLanes.markVerified(driver) + return driver + } catch { + sessionLanes.close(database: database, for: connectionId, supersedingOpens: false) + /// Recorded before the reopen, which can fail or be superseded: the transaction is gone + /// either way, and whatever runs on this database next has to hear about it. + if !held.permitsAppTransaction { + sessionLanes.markTransactionLost(database: database, for: connectionId) + } + let generation = sessionLanes.generation(for: connectionId) + let reopened = try await sessionLanes.open( + DatabaseScope(connectionId: connectionId, database: database, schema: nil) + ) + guard sessionLanes.generation(for: connectionId) == generation, + activeSessions[connectionId]?.resolvedBrowseDatabase != database + else { + reopened.disconnect() + throw DatabaseError.notConnected } + sessionLanes.park(reopened, database: database, for: connectionId) + sessionLanes.markVerified(reopened) + return reopened } } + static func parkedTransactionLostMessage(database: String) -> String { + String( + format: String( + localized: "The connection to %@ was lost while it waited, and the server rolled back its open transaction. It has been reconnected; run the statements again." + ), + database + ) + } + /// Moves the shared driver onto the scope. It writes no session state, so the /// sidebar and the toolbar do not follow a tab's operation. /// @@ -375,7 +490,9 @@ extension DatabaseManager { let databaseType = session.connection.type if !scope.isServerScoped, pluginManager.supportsDatabaseSwitching(for: databaseType) { if pluginManager.requiresReconnectForDatabaseSwitch(for: databaseType) { - guard scope.database == session.resolvedBrowseDatabase else { + guard scope.database == session.resolvedBrowseDatabase + || sessionLanes.parkedDriver(for: scope.connectionId, database: scope.database) === driver + else { throw DatabaseError.queryFailed( String( format: String( diff --git a/TablePro/Core/Database/DatabaseManager+Sessions.swift b/TablePro/Core/Database/DatabaseManager+Sessions.swift index 40c0e1a81d..a3f3428808 100644 --- a/TablePro/Core/Database/DatabaseManager+Sessions.swift +++ b/TablePro/Core/Database/DatabaseManager+Sessions.swift @@ -407,7 +407,11 @@ extension DatabaseManager { } if pm?.capabilities.requiresReconnectForDatabaseSwitch == true { - try await reconnectOntoDatabase(database, for: connectionId) + if let session = session(for: connectionId), usesDatabaseLanes(session) { + try await moveBrowsingOntoLane(database, for: connectionId) + } else { + try await reconnectOntoDatabase(database, for: connectionId) + } } else if driver is PluginDriverAdapter { let grouping = pm?.schema.databaseGroupingStrategy ?? .byDatabase let sessionStartedAt = session(for: connectionId)?.connectedAt @@ -448,6 +452,118 @@ extension DatabaseManager { AppEvents.shared.browseContainerChanged.send(connectionId) } + /// Browses `database` on its own connection, for an engine that cannot change database on a live + /// connection but can open a second one to the same server. + /// + /// The connection being left is parked, not closed, so its transaction, temp tables and settings + /// stay where the user left them, and coming back to it is a swap rather than a connect. Only a + /// database never browsed before opens a connection, and that open happens before anything moves: + /// a server that refuses it (no access, too many connections) leaves the session as it was. + /// + /// `connection.database` follows the browsed database, as it did when a switch reconnected, so + /// every reconnect path keeps dialing the database on screen. + private func moveBrowsingOntoLane(_ database: String, for connectionId: UUID) async throws { + guard let session = session(for: connectionId), let leavingDriver = session.driver else { + throw DatabaseError.notConnected + } + let leavingDatabase = session.resolvedBrowseDatabase + guard database != leavingDatabase else { return } + /// A transport being rebuilt ends every connection dialed through it, the one this would + /// promote included, so the switch waits for the connection to come back. + guard !MetadataConnectionPool.shared.isReplacingTransport(for: connectionId) else { + throw DatabaseError.connectionFailed(Self.reconnectingSwitchMessage) + } + let sessionStartedAt = session.connectedAt + let generation = sessionLanes.beginSwitch(for: connectionId) + + let target: DatabaseDriver + let freshlyOpened: Bool + if let parkedDriver = await answeringParkedDriver(database: database, for: connectionId) { + target = parkedDriver + freshlyOpened = false + } else { + target = try await sessionLanes.open( + DatabaseScope(connectionId: connectionId, database: database, schema: nil) + ) + freshlyOpened = true + } + guard sessionLanes.isCurrent(generation, for: connectionId), + !MetadataConnectionPool.shared.isReplacingTransport(for: connectionId), + self.session(for: connectionId)?.connectedAt == sessionStartedAt, + self.session(for: connectionId)?.driver === leavingDriver + else { + if freshlyOpened { target.disconnect() } + throw CancellationError() + } + + _ = sessionLanes.unpark(database: database, for: connectionId) + sessionLanes.park(leavingDriver, database: leavingDatabase, for: connectionId) + /// A parked connection may have died while nobody used it, so the first turn on it asks the + /// server again rather than trusting the answer the previous browsed connection earned. + if freshlyOpened { + lastVerifiedAt[connectionId] = Date() + } else { + lastVerifiedAt.removeValue(forKey: connectionId) + } + let targetSchema = (target as? SchemaSwitchable)?.currentSchema + updateSession(connectionId) { session in + session.connection.database = database + session.effectiveConnection?.database = database + session.browseDatabase = database + session.browseSchema = targetSchema + session.driver = target + session.status = .connected + } + /// The connection on screen is now one that answered, whatever the check on the database + /// just left concluded about that one. + if freshlyOpened || sessionLanes.isFresh(target) { + markSessionLive(connectionId) + } + /// The saved schema is what the next launch restores on this connection, and the one on + /// file belongs to the database just left. + appSettingsStorage.saveLastSchema(targetSchema, for: connectionId) + await SchemaService.shared.invalidate(connectionId: connectionId) + Self.logger.info( + "switchDatabase moved onto a database connection conn=\(connectionId, privacy: .public) reused=\(!freshlyOpened)" + ) + } + + /// The connection parked for `database`, if it still answers. One that died is closed, and if it + /// was holding a transaction, the next work on that database is told the server rolled it back. + /// A turn running on it right now is answer enough, and asking would wait for that turn. + private func answeringParkedDriver(database: String, for connectionId: UUID) async -> DatabaseDriver? { + let key = SessionDriverGate.Key(connectionId: connectionId, database: database) + guard var parked = sessionLanes.parkedDriver(for: connectionId, database: database) else { return nil } + /// A check already running decides for this switch too: once it ends it has either confirmed + /// the driver or parked a replacement, and that is what gets promoted. + if sessionLanes.isVerifying(parked) { + await sessionLanes.waitForVerification(of: parked) + guard let settled = sessionLanes.parkedDriver(for: connectionId, database: database) else { return nil } + parked = settled + } + if sessionLanes.isFresh(parked) || sessionDriverGate.isHeld(key) { return parked } + /// Read before the ping, which on a closed socket leaves libpq reporting no state at all. + let held = await parked.heldSessionTransactionState() + do { + try await sessionDriverGate.withExclusiveAccess(key) { + try await parked.ping() + } + sessionLanes.markVerified(parked) + return parked + } catch { + guard sessionLanes.parkedDriver(for: connectionId, database: database) === parked else { return nil } + sessionLanes.close(database: database, for: connectionId, supersedingOpens: false) + if !held.permitsAppTransaction { + sessionLanes.markTransactionLost(database: database, for: connectionId) + } + return nil + } + } + + static let reconnectingSwitchMessage = String( + localized: "The connection is reconnecting. Switch databases again once it is back." + ) + /// Reopens the connection on `database`, for an engine that cannot change database on a live /// connection. /// @@ -535,12 +651,18 @@ extension DatabaseManager { /// ping skips at its `queriesInFlight` guard instead of entering a driver that is not /// thread-safe alongside this. Holding `sessionDriverGate` is not enough on its own: the /// ping never asks for that gate. - try await sessionDriverGate.withExclusiveAccess(connectionId) { + let gateKey = browsedGateKey(for: connectionId) + try await sessionDriverGate.withExclusiveAccess(gateKey) { try await trackOperation(sessionId: connectionId) { try Task.checkCancellation() guard session(for: connectionId)?.connectedAt == sessionStartedAt else { throw CancellationError() } + /// A database switch while this waited moved the browsed connection to another + /// database, whose schema this was never asked to change. + if let database = gateKey.database, session(for: connectionId)?.resolvedBrowseDatabase != database { + throw CancellationError() + } guard let schemaDriver = driver(for: connectionId) as? SchemaSwitchable else { throw DatabaseError.notConnected } @@ -904,6 +1026,8 @@ extension DatabaseManager { internal func removeSessionEntry(for connectionId: UUID) { activeSessions.removeValue(forKey: connectionId) MetadataConnectionPool.shared.closeAll(connectionId: connectionId) + sessionLanes.closeAll(for: connectionId) + monitorPingedDrivers.removeValue(forKey: connectionId) sessionDriverGate.drain(connectionId: connectionId) connectionStatusVersions.removeValue(forKey: connectionId) forgetVerification(for: connectionId) diff --git a/TablePro/Core/Database/DatabaseManager+Tunnel.swift b/TablePro/Core/Database/DatabaseManager+Tunnel.swift index c14a569369..0d5e527d0e 100644 --- a/TablePro/Core/Database/DatabaseManager+Tunnel.swift +++ b/TablePro/Core/Database/DatabaseManager+Tunnel.swift @@ -161,6 +161,7 @@ extension DatabaseManager { await stopHealthMonitor(for: connectionId) + await sessionLanes.closeAllNotingLostTransactions(for: connectionId) activeSessions[connectionId]?.driver?.disconnect() updateSession(connectionId) { session in session.driver = nil diff --git a/TablePro/Core/Database/DatabaseManager+Verification.swift b/TablePro/Core/Database/DatabaseManager+Verification.swift index 5fe83ebd55..5a5d04fe93 100644 --- a/TablePro/Core/Database/DatabaseManager+Verification.swift +++ b/TablePro/Core/Database/DatabaseManager+Verification.swift @@ -156,7 +156,10 @@ extension DatabaseManager { } catch { guard activeSessions[connectionId]?.driver === driver else { return } Self.logger.info("Connection \(connectionId) did not answer before use, reconnecting") - let outcome = await performHealthMonitorReconnect(connectionId: connectionId) + let outcome = await performHealthMonitorReconnect( + connectionId: connectionId, + failedDriver: ObjectIdentifier(driver) + ) guard case .retry(let failure) = outcome else { return } /// The reconnect disconnected the installed driver before it failed, so the session is /// holding a handle that cannot work. Saying so is the whole point: `ensureConnected` diff --git a/TablePro/Core/Database/DatabaseManager.swift b/TablePro/Core/Database/DatabaseManager.swift index 2b8426516f..7eb29c1882 100644 --- a/TablePro/Core/Database/DatabaseManager.swift +++ b/TablePro/Core/Database/DatabaseManager.swift @@ -127,6 +127,23 @@ final class DatabaseManager: ObservableObject { /// their pins and each run against the other's database. internal let sessionDriverGate = SessionDriverGate() + /// The connections a session keeps on databases it is not browsing. See `SessionLanes`. + internal let sessionLanes = SessionLanes() + + /// The driver each connection's health monitor last pinged, so the reconnect that follows a + /// failed ping acts only on that driver and never on one a database switch put in its place. + /// Held, not just identified: a closed driver that was freed would hand its address, and with it + /// the fence, to the next driver allocated. + internal var monitorPingedDrivers: [UUID: DatabaseDriver] = [:] + + internal func notePinged(_ driver: DatabaseDriver, for connectionId: UUID) { + monitorPingedDrivers[connectionId] = driver + } + + internal func pingedDriver(for connectionId: UUID) -> ObjectIdentifier? { + monitorPingedDrivers[connectionId].map { ObjectIdentifier($0) } + } + /// The drivers each connection is currently executing user SQL on, keyed by an /// operation token so a finishing operation can only release its own handle. Stop /// reaches the right one even when a cross-database tab runs on a pooled connection. diff --git a/TablePro/Core/Database/SessionLanes.swift b/TablePro/Core/Database/SessionLanes.swift new file mode 100644 index 0000000000..257960ac5b --- /dev/null +++ b/TablePro/Core/Database/SessionLanes.swift @@ -0,0 +1,194 @@ +// +// SessionLanes.swift +// TablePro +// + +import Foundation +import os + +/// The user connections a session keeps open on databases other than the one it is browsing. +/// +/// An engine that cannot change database on a live connection used to reconnect on every switch, +/// which threw away the transaction, temp tables and settings of the database being left, and made +/// every click between two database entries a whole connect. Each database the user has browsed now +/// keeps its own connection: the browsed one is `ConnectionSession.driver`, the others are parked +/// here, and a switch moves a driver between the two without closing anything. +@MainActor +internal final class SessionLanes { + typealias Opener = @MainActor (DatabaseScope) async throws -> DatabaseDriver + + private static let logger = Logger(subsystem: "com.TablePro", category: "SessionLanes") + + private var parked: [UUID: [String: DatabaseDriver]] = [:] + private var generations: [UUID: Int] = [:] + /// Holds the driver weakly beside its identifier: a driver promoted and then closed by the + /// session leaves the lanes without passing through them, and the next driver allocated at its + /// address must not inherit its check. + private struct Check { + weak var driver: AnyObject? + let at: Date + } + + private var verifiedAt: [ObjectIdentifier: Check] = [:] + /// Drivers being checked before use. A turn held on one of these is not a query proving the + /// driver answers, so a switch back waits for the check instead of promoting it on trust. + private var verifying: [ObjectIdentifier: [CheckedContinuation]] = [:] + /// Databases whose connection died holding a transaction and was replaced. The next turn there + /// is refused once, so work meant to continue that transaction does not carry on without it. + private var lostTransactions: [UUID: Set] = [:] + + /// Opens the connection for a database the session has not browsed yet. Replaced under test, + /// where plugins never load and no server is there to dial. + internal var opener: Opener + + internal init(opener: Opener? = nil) { + self.opener = opener ?? { scope in + try await MetadataConnectionPool.openServerDriver(for: scope, purpose: .session) + } + } + + internal func parkedDriver(for connectionId: UUID, database: String) -> DatabaseDriver? { + parked[connectionId]?[database] + } + + internal func parkedDatabases(for connectionId: UUID) -> [String] { + parked[connectionId].map { Array($0.keys) } ?? [] + } + + internal func isParked(_ driver: DatabaseDriver, for connectionId: UUID) -> Bool { + parked[connectionId]?.values.contains { $0 === driver } ?? false + } + + /// Keeps a driver for its database. A different driver already parked there is closed, because + /// two connections for one database would split the user's session between them. + internal func park(_ driver: DatabaseDriver, database: String, for connectionId: UUID) { + if let existing = parked[connectionId]?[database], existing !== driver { + existing.disconnect() + } + parked[connectionId, default: [:]][database] = driver + } + + /// Takes a parked driver out to become the browsed one. + internal func unpark(database: String, for connectionId: UUID) -> DatabaseDriver? { + let driver = parked[connectionId]?.removeValue(forKey: database) + if parked[connectionId]?.isEmpty == true { + parked.removeValue(forKey: connectionId) + } + return driver + } + + /// Closes the connection kept for one database, for a closed rail entry or a database that is + /// about to be renamed or dropped. A reopen of that database still in flight is superseded, so it + /// cannot land afterwards and park a connection for a database that was closed or is going away. + /// The switch and the before-use check that close a dead connection on their own way to a new one + /// leave their own open standing. + internal func close(database: String, for connectionId: UUID, supersedingOpens: Bool = true) { + if supersedingOpens { + generations[connectionId, default: 0] &+= 1 + } + guard let driver = unpark(database: database, for: connectionId) else { return } + Self.logger.info("closing parked connection connId=\(connectionId, privacy: .public)") + verifiedAt.removeValue(forKey: ObjectIdentifier(driver)) + driver.disconnect() + } + + /// Closes every parked connection of a session. A disconnect ends them, and so does a rebuilt + /// transport: they were dialed through the tunnel port that is going away. Opens still in flight + /// lose their generation, so one that lands afterwards closes its own driver. + internal func closeAll(for connectionId: UUID, keepingLostTransactions: Bool = false) { + generations[connectionId, default: 0] &+= 1 + let drivers = parked.removeValue(forKey: connectionId) ?? [:] + if !keepingLostTransactions { + lostTransactions.removeValue(forKey: connectionId) + } + for driver in drivers.values { + verifiedAt.removeValue(forKey: ObjectIdentifier(driver)) + driver.disconnect() + } + if !drivers.isEmpty { + Self.logger.info("closed \(drivers.count) parked connection(s) connId=\(connectionId, privacy: .public)") + } + } + + internal func generation(for connectionId: UUID) -> Int { + generations[connectionId, default: 0] + } + + /// Starts a switch, superseding any switch of the same session still opening its connection, so + /// two quick clicks land on the second database rather than on whichever open finished last. + internal func beginSwitch(for connectionId: UUID) -> Int { + generations[connectionId, default: 0] &+= 1 + return generations[connectionId, default: 0] + } + + internal func isCurrent(_ generation: Int, for connectionId: UUID) -> Bool { + generations[connectionId, default: 0] == generation + } + + internal func open(_ scope: DatabaseScope) async throws -> DatabaseDriver { + try await opener(scope) + } + + /// Whether a parked driver answered recently enough to skip asking again before a turn. + internal func isFresh(_ driver: DatabaseDriver) -> Bool { + /// A driver that has since reported its connection gone has answered the question already. + guard !driver.hasLostConnection, + let check = verifiedAt[ObjectIdentifier(driver)], check.driver === driver + else { return false } + return ConnectionHealthCheck.isFresh(check.at) + } + + internal func markVerified(_ driver: DatabaseDriver) { + verifiedAt = verifiedAt.filter { $0.value.driver != nil } + verifiedAt[ObjectIdentifier(driver)] = Check(driver: driver, at: Date()) + } + + internal func beginVerifying(_ driver: DatabaseDriver) { + let id = ObjectIdentifier(driver) + verifying[id] = verifying[id] ?? [] + } + + internal func endVerifying(_ driver: DatabaseDriver) { + verifying.removeValue(forKey: ObjectIdentifier(driver))?.forEach { $0.resume() } + } + + internal func isVerifying(_ driver: DatabaseDriver) -> Bool { + verifying[ObjectIdentifier(driver)] != nil + } + + /// Returns once the check running on `driver` ends, without waiting for the work queued behind + /// it on the same turn. + internal func waitForVerification(of driver: DatabaseDriver) async { + let id = ObjectIdentifier(driver) + guard verifying[id] != nil else { return } + await withCheckedContinuation { continuation in + verifying[id, default: []].append(continuation) + } + } + + internal func markTransactionLost(database: String, for connectionId: UUID) { + lostTransactions[connectionId, default: []].insert(database) + } + + internal func hasTransactionLoss(database: String, for connectionId: UUID) -> Bool { + lostTransactions[connectionId]?.contains(database) ?? false + } + + /// Closes every parked connection when the transport under them is rebuilt, first noting each + /// one that was holding a transaction, so the next work on that database is told the server + /// rolled it back instead of quietly starting over on a new connection. + internal func closeAllNotingLostTransactions(for connectionId: UUID) async { + for (database, driver) in parked[connectionId] ?? [:] { + let held = await driver.heldSessionTransactionState() + if !held.permitsAppTransaction { + lostTransactions[connectionId, default: []].insert(database) + } + } + closeAll(for: connectionId, keepingLostTransactions: true) + } + + /// Reports a lost transaction once, clearing it. + internal func takeTransactionLoss(database: String, for connectionId: UUID) -> Bool { + lostTransactions[connectionId]?.remove(database) != nil + } +} diff --git a/TablePro/Core/MCP/MCPConnectionBridge+Server.swift b/TablePro/Core/MCP/MCPConnectionBridge+Server.swift index 82d7aab79a..780734e067 100644 --- a/TablePro/Core/MCP/MCPConnectionBridge+Server.swift +++ b/TablePro/Core/MCP/MCPConnectionBridge+Server.swift @@ -546,6 +546,10 @@ extension MCPConnectionBridge { } func dropDatabase(connectionId: UUID, name: String) async throws -> JsonValue { + await MainActor.run { + MetadataConnectionPool.shared.closeAll(connectionId: connectionId, database: name) + DatabaseManager.shared.sessionLanes.close(database: name, for: connectionId) + } let (driver, _) = try await resolveDriver(connectionId) try await DatabaseManager.shared.trackOperation(sessionId: connectionId) { try await driver.dropDatabase(name: name) diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index d41cd3ca6e..fc2c5ce8cc 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -29,6 +29,10 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { private let tableEditDialect: TableEditDialect? private let nameHazards = OSAllocatedUnfairLock<[String]>(initialState: []) + /// Whether this import opened the transaction it is in. A session connection can already hold + /// the user's own: a `BEGIN` there would join it, and the import's `COMMIT` would commit work the + /// user never asked to commit, so the import then runs inside it and leaves ending it to them. + private let ownsTransaction = OSAllocatedUnfairLock(initialState: false) private static let logger = Logger(subsystem: "com.TablePro", category: "ImportDataSinkAdapter") @@ -253,15 +257,25 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { } func beginTransaction() async throws { + let owner = WriteTransactionOwner.resolve( + supportsTransactions: driver.supportsTransactions, + sessionState: await driver.heldSessionTransactionState() + ) + ownsTransaction.withLock { $0 = owner.opensTransaction } + guard owner.opensTransaction else { return } try await driver.beginTransaction(mode: .readWrite) } func commitTransaction() async throws { + guard ownsTransaction.withLock({ $0 }) else { return } try await driver.commitTransaction() + ownsTransaction.withLock { $0 = false } } func rollbackTransaction() async throws { + guard ownsTransaction.withLock({ $0 }) else { return } try await driver.rollbackTransaction() + ownsTransaction.withLock { $0 = false } } func disableForeignKeyChecks() async throws { diff --git a/TablePro/Core/Services/Infrastructure/WorkspaceCloseAction.swift b/TablePro/Core/Services/Infrastructure/WorkspaceCloseAction.swift index 911006710f..191152ffbc 100644 --- a/TablePro/Core/Services/Infrastructure/WorkspaceCloseAction.swift +++ b/TablePro/Core/Services/Infrastructure/WorkspaceCloseAction.swift @@ -103,9 +103,9 @@ internal enum WorkspaceCloseAction { return } /// The entry goes now, before the connection leaves the container, because leaving it is a - /// reconnect and a schema reload on every engine that cannot change database on a live - /// connection: waiting for that left the row the user just closed sitting there for seconds - /// while the window loaded somewhere else. `beginClosing` is what lets the strip drop the + /// schema reload, and on an engine that can neither change database on a live connection nor + /// open a second one, a reconnect: waiting for that left the row the user just closed sitting + /// there for seconds while the window loaded somewhere else. `beginClosing` is what lets the strip drop the /// browse cursor's own row early, and the cursor follows underneath. if !victims.isEmpty { closeTabs(victims.map(\.id), across: coordinators) @@ -153,6 +153,9 @@ internal enum WorkspaceCloseAction { } } landOnRemainingTab(after: workspace, among: containers, coordinator: coordinator) + /// The database keeps its own connection while its entry is listed; closing the entry is + /// what ends it, and the connection has already moved off it above. + DatabaseManager.shared.sessionLanes.close(database: workspace.container, for: workspace.connectionId) } /// Shown, then asked, for the same reason a connection close reveals itself first: an alert diff --git a/TablePro/Core/Services/Query/MetadataConnectionPool.swift b/TablePro/Core/Services/Query/MetadataConnectionPool.swift index d26b9a8bf9..2c67925426 100644 --- a/TablePro/Core/Services/Query/MetadataConnectionPool.swift +++ b/TablePro/Core/Services/Query/MetadataConnectionPool.swift @@ -114,7 +114,7 @@ final class MetadataConnectionPool { private init(openDriver: DriverOpener? = nil) { self.openDriver = openDriver ?? { scope in - try await MetadataConnectionPool.openSessionDriver(for: scope) + try await MetadataConnectionPool.openServerDriver(for: scope, purpose: .metadata) } } @@ -219,11 +219,13 @@ final class MetadataConnectionPool { internal func transportWaiterCount(for connectionId: UUID) -> Int { transportWaiters[connectionId]?.count ?? 0 } + #endif + /// Whether the connection's transport is being rebuilt, which ends every connection dialed + /// through it, so nothing new should be opened or promoted onto it until that is over. internal func isReplacingTransport(for connectionId: UUID) -> Bool { transportReplacements[connectionId] != nil } - #endif private func releaseEntry(_ entry: Entry, connectionId: UUID) { entry.inFlightCount -= 1 @@ -406,7 +408,11 @@ final class MetadataConnectionPool { waiter.continuation.resume(throwing: CancellationError()) } - private static func openSessionDriver(for scope: DatabaseScope) async throws -> DatabaseDriver { + /// Opens a second connection to the session's server, on `scope`'s database, through the + /// transport the session already holds. Every tunnel manager closes its tunnel before building a + /// new one, so dialing the session's effective endpoint is what keeps the other connections up. + /// A database lane opens its user connection here too, as `.session`. + internal static func openServerDriver(for scope: DatabaseScope, purpose: DriverPurpose) async throws -> DatabaseDriver { guard let session = DatabaseManager.shared.session(for: scope.connectionId) else { throw DatabaseError.notConnected } @@ -422,7 +428,7 @@ final class MetadataConnectionPool { connection.database = plan.connectDatabase let preparedConfiguration = try await DatabaseDriverFactory.prepareConfiguration( for: connection, - purpose: .metadata + purpose: purpose ) let deadline = ConnectionDeadline(configuredSeconds: session.connection.connectTimeoutSeconds) let timeoutEndpoint = ConnectionTimeoutEndpoint.database( @@ -436,7 +442,7 @@ final class MetadataConnectionPool { for: connection, passwordOverride: session.cachedPassword, awaitPlugins: true, - purpose: .metadata, + purpose: purpose, deadline: deadline, timeoutEndpoint: timeoutEndpoint, effectiveQueryTimeoutSeconds: session.effectiveQueryTimeoutSeconds, diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift index 472df6c414..b81629b8fa 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Navigation.swift @@ -686,6 +686,7 @@ extension MainContentCoordinator { : PluginManager.shared.containerEntityName(for: connection.type) let name = target.name let kind = target.kind + let connectionId = connectionId try await DatabaseManager.shared.runContainerOperation( description: String(format: String(localized: "Drop %1$@ \"%2$@\""), entity, name), kind: .destructiveQuery, @@ -698,7 +699,15 @@ extension MainContentCoordinator { isConfirmationPreCleared: true ) { driver in switch kind { - case .database: try await driver.dropDatabase(name: name) + case .database: + /// PostgreSQL refuses to drop a database while a backend is attached to it, and the + /// connection kept for it and the pooled ones are such backends. Closed only here, + /// once the drop is authorized: a refused drop must not end the session on it. + await MainActor.run { + MetadataConnectionPool.shared.closeAll(connectionId: connectionId, database: name) + DatabaseManager.shared.sessionLanes.close(database: name, for: connectionId) + } + try await driver.dropDatabase(name: name) case .schema: try await driver.dropSchema(name: name) } } diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+Rename.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+Rename.swift index 9ed7d5835b..088d184a19 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+Rename.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+Rename.swift @@ -82,6 +82,7 @@ extension MainContentCoordinator { /// is, so its leases on that database are closed first. if let database = ref.database { MetadataConnectionPool.shared.closeAll(connectionId: connectionId, database: database) + DatabaseManager.shared.sessionLanes.close(database: database, for: connectionId) } guard let scope = browseScope else { throw DatabaseError.notConnected } let route = DatabaseManager.shared.executionRoute(for: scope) diff --git a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift index a803aa9818..fcfa01ee5e 100644 --- a/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift +++ b/TableProTests/Core/Autocomplete/SQLSchemaProviderTests.swift @@ -59,6 +59,14 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen return serverOutputToReturn } + var sessionTransactionStateToReturn: PluginSessionTransactionState = .unknown + /// libpq reports no transaction state once a check has found the socket closed. + var pingFailureForgetsTransactionState = false + + func sessionTransactionState() async -> PluginSessionTransactionState { + sessionTransactionStateToReturn + } + init(connection: DatabaseConnection = TestFixtures.makeConnection()) { self.connection = connection } @@ -94,6 +102,9 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen try? await Task.sleep(nanoseconds: UInt64(pingDelaySeconds * 1_000_000_000)) } if let pingError { + if pingFailureForgetsTransactionState { + sessionTransactionStateToReturn = .unknown + } throw pingError } } @@ -276,8 +287,10 @@ final class MockDatabaseDriver: DatabaseDriver, SchemaSwitchable, @unchecked Sen func createDatabase(name: String, charset: String, collation: String?) async throws {} func cancelQuery() throws { cancelQueryCallCount += 1 } - func beginTransaction() async throws {} - func commitTransaction() async throws {} + var beginTransactionCallCount = 0 + var commitTransactionCallCount = 0 + func beginTransaction() async throws { beginTransactionCallCount += 1 } + func commitTransaction() async throws { commitTransactionCallCount += 1 } func rollbackTransaction() async throws {} func switchSchema(to schema: String) async throws { diff --git a/TableProTests/Core/Database/DatabaseSwitchLeaseOrderingTests.swift b/TableProTests/Core/Database/DatabaseSwitchLeaseOrderingTests.swift index 97ed175e0a..41e272e764 100644 --- a/TableProTests/Core/Database/DatabaseSwitchLeaseOrderingTests.swift +++ b/TableProTests/Core/Database/DatabaseSwitchLeaseOrderingTests.swift @@ -41,8 +41,10 @@ struct DatabaseSwitchLeaseOrderingTests { /// A type that reopens its connection to change database, and whose driver plugin is not /// registered, so every switch reaches the reconnect and fails there. `pools` says whether it can - /// open a second connection for a database the session has left. - private func registerTypeIfNeeded(_ typeId: String = Self.typeId, pools: Bool = true) { + /// open a second connection for a database the session has left; one that can switches by + /// keeping a connection per database instead (`SessionLanesTests`), so the reconnect needs one + /// that cannot. + private func registerTypeIfNeeded(_ typeId: String = Self.typeId, pools: Bool = false) { guard PluginMetadataRegistry.shared.snapshot(forRegisteredTypeId: typeId) == nil else { return } let defaults = PluginMetadataSnapshot.CapabilityFlags.defaults var capabilities = PluginMetadataSnapshot.CapabilityFlags( @@ -90,11 +92,15 @@ struct DatabaseSwitchLeaseOrderingTests { PluginMetadataRegistry.shared.unregister(typeId: Self.typeId) } - /// Holds the connection's driver until `release` opens, and returns only once it holds it. - private func holdDriver(_ connectionId: UUID, until release: Latch) async -> Task { + /// Holds the connection's driver until `release` opens, and returns only once it holds it. A + /// connection that keeps one driver per database takes its turns per database, so `database` + /// names which one to hold. + private func holdDriver(_ connectionId: UUID, database: String? = nil, until release: Latch) async -> Task { let acquired = Latch() let holder = Task { @MainActor in - try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess(connectionId) { + try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess( + SessionDriverGate.Key(connectionId: connectionId, database: database) + ) { acquired.open() await release.wait() } @@ -355,7 +361,7 @@ struct DatabaseSwitchLeaseOrderingTests { defer { cleanUpTableRead(connection.id) } let app = appScope(connection) let release = Latch() - let holder = await holdDriver(connection.id, until: release) + let holder = await holdDriver(connection.id, database: "app", until: release) let read = Task { @MainActor in try await DatabaseManager.shared.withTableReadDriver(scope: app, cancellation: .cancellableRead(DriverLeaseOwner())) { driver in @@ -380,7 +386,7 @@ struct DatabaseSwitchLeaseOrderingTests { defer { cleanUpTableRead(connection.id) } let app = appScope(connection) let release = Latch() - let holder = await holdDriver(connection.id, until: release) + let holder = await holdDriver(connection.id, database: "app", until: release) let ran = LeaseRecord() let lease = Task { @MainActor in @@ -414,7 +420,7 @@ struct DatabaseSwitchLeaseOrderingTests { defer { cleanUpTableRead(connection.id) } let app = appScope(connection) let release = Latch() - let holder = await holdDriver(connection.id, until: release) + let holder = await holdDriver(connection.id, database: "app", until: release) let running = LeaseRecord() let finish = Latch() @@ -434,7 +440,9 @@ struct DatabaseSwitchLeaseOrderingTests { let probe = LeaseRecord() let prober = Task { @MainActor in - try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess(connection.id) { + try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess( + SessionDriverGate.Key(connectionId: connection.id, database: "app") + ) { probe.didRun = true } } @@ -455,7 +463,7 @@ struct DatabaseSwitchLeaseOrderingTests { defer { cleanUpTableRead(connection.id) } let app = appScope(connection) let release = Latch() - let holder = await holdDriver(connection.id, until: release) + let holder = await holdDriver(connection.id, database: "app", until: release) let ran = LeaseRecord() let read = Task { @MainActor in diff --git a/TableProTests/Core/Database/SessionLanesTests.swift b/TableProTests/Core/Database/SessionLanesTests.swift new file mode 100644 index 0000000000..46ba3c884e --- /dev/null +++ b/TableProTests/Core/Database/SessionLanesTests.swift @@ -0,0 +1,450 @@ +// +// SessionLanesTests.swift +// TableProTests +// +// An engine that cannot change database on a live connection used to reconnect on every switch +// between two database entries, dropping the transaction, temp tables and settings of the database +// it left. Each browsed database now keeps its own connection, and a switch moves between them. +// + +import Foundation +@testable import TablePro +import TableProPluginKit +import Testing + +@Suite("Session lanes", .serialized) +@MainActor +struct SessionLanesTests { + /// Reopens its connection to change database and can open a second one, like PostgreSQL. + private static let typeId = "SessionLaneFake" + + private final class Opener { + var opened: [String: MockDatabaseDriver] = [:] + var openCount = 0 + var failure: Error? + var schemaForNextOpen: String? + + func open(_ scope: DatabaseScope) async throws -> DatabaseDriver { + openCount += 1 + if let failure { throw failure } + let driver = MockDatabaseDriver() + driver.currentSchema = schemaForNextOpen + opened[scope.database] = driver + return driver + } + } + + private struct Harness { + let connection: DatabaseConnection + let home: MockDatabaseDriver + let opener: Opener + } + + private func registerTypeIfNeeded() { + guard PluginMetadataRegistry.shared.snapshot(forRegisteredTypeId: Self.typeId) == nil else { return } + let defaults = PluginMetadataSnapshot.CapabilityFlags.defaults + let capabilities = PluginMetadataSnapshot.CapabilityFlags( + supportsSchemaSwitching: true, + supportsImport: defaults.supportsImport, + supportsExport: defaults.supportsExport, + supportsSSH: defaults.supportsSSH, + supportsSSL: defaults.supportsSSL, + supportsCascadeDrop: defaults.supportsCascadeDrop, + supportsForeignKeyDisable: defaults.supportsForeignKeyDisable, + supportsReadOnlyMode: defaults.supportsReadOnlyMode, + supportsQueryProgress: defaults.supportsQueryProgress, + requiresReconnectForDatabaseSwitch: true, + supportsDropDatabase: defaults.supportsDropDatabase + ) + let snapshot = PluginMetadataSnapshot( + displayName: Self.typeId, iconName: "cylinder", defaultPort: 1_234, + requiresAuthentication: true, supportsForeignKeys: true, supportsSchemaEditing: true, + isDownloadable: false, primaryUrlScheme: "sessionlanefake", parameterStyle: .questionMark, + navigationModel: .standard, explainVariants: [], pathFieldRole: .database, + supportsHealthMonitor: false, urlSchemes: ["sessionlanefake"], postConnectActions: [], + brandColorHex: "#000000", queryLanguageName: "SQL", editorLanguage: .sql, + connectionMode: .network, supportsDatabaseSwitching: true, + capabilities: capabilities, schema: .defaults, editor: .defaults, connection: .defaults + ) + PluginMetadataRegistry.shared.register(snapshot: snapshot, forTypeId: Self.typeId) + } + + /// A connected session browsing `app` on `home`, with lane opens answered by `opener`. + private func makeHarness() -> Harness { + registerTypeIfNeeded() + var connection = TestFixtures.makeConnection(database: "app") + connection.type = DatabaseType(rawValue: Self.typeId) + let home = MockDatabaseDriver(connection: connection) + var session = ConnectionSession(connection: connection, driver: home) + session.status = .connected + session.browseDatabase = "app" + DatabaseManager.shared.injectSession(session, for: connection.id) + let opener = Opener() + let connectionId = connection.id + /// The lanes are shared with every suite running alongside this one, so only this + /// connection's opens are answered here. + DatabaseManager.shared.sessionLanes.opener = { scope in + guard scope.connectionId == connectionId else { + return try await MetadataConnectionPool.openServerDriver(for: scope, purpose: .session) + } + return try await opener.open(scope) + } + return Harness(connection: connection, home: home, opener: opener) + } + + private func cleanUp(_ harness: Harness) { + DatabaseManager.shared.removeSession(for: harness.connection.id) + DatabaseManager.shared.sessionLanes.opener = { scope in + try await MetadataConnectionPool.openServerDriver(for: scope, purpose: .session) + } + AppSettingsStorage.shared.saveLastDatabase(nil, for: harness.connection.id) + AppSettingsStorage.shared.saveLastSchema(nil, for: harness.connection.id) + PluginMetadataRegistry.shared.unregister(typeId: Self.typeId) + } + + private func session(_ harness: Harness) -> ConnectionSession? { + DatabaseManager.shared.session(for: harness.connection.id) + } + + @Test("Switching database opens the new one and keeps the one left, without reconnecting") + func switchKeepsTheConnectionLeft() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + + let logs = try #require(harness.opener.opened["logs"]) + #expect(harness.opener.openCount == 1) + #expect(session(harness)?.driver === logs) + #expect(session(harness)?.status == .connected) + #expect(session(harness)?.resolvedBrowseDatabase == "logs") + #expect(session(harness)?.connection.database == "logs") + #expect(harness.home.disconnectCallCount == 0) + #expect(DatabaseManager.shared.sessionLanes.parkedDriver(for: harness.connection.id, database: "app") === harness.home) + } + + @Test("Switching back reuses the connection the database already had") + func switchingBackReusesTheConnection() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + try await DatabaseManager.shared.switchDatabase(to: "app", for: harness.connection.id, persist: false) + + let logs = try #require(harness.opener.opened["logs"]) + #expect(harness.opener.openCount == 1) + #expect(session(harness)?.driver === harness.home) + #expect(harness.home.disconnectCallCount == 0) + #expect(logs.disconnectCallCount == 0) + #expect(DatabaseManager.shared.sessionLanes.parkedDriver(for: harness.connection.id, database: "logs") === logs) + } + + @Test("A database the server refuses leaves the session where it was") + func refusedOpenChangesNothing() async { + let harness = makeHarness() + defer { cleanUp(harness) } + harness.opener.failure = DatabaseError.connectionFailed("too many connections") + + await #expect(throws: (any Error).self) { + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + } + + #expect(session(harness)?.driver === harness.home) + #expect(session(harness)?.resolvedBrowseDatabase == "app") + #expect(session(harness)?.connection.database == "app") + #expect(DatabaseManager.shared.sessionLanes.parkedDatabases(for: harness.connection.id).isEmpty) + #expect(harness.home.disconnectCallCount == 0) + } + + @Test("Work on a database the user switched away from runs on that database's own connection") + func workOnALeftDatabaseRunsOnItsConnection() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + + let route = DatabaseManager.shared.executionRoute(for: scope) + let ranOn = try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: route, cancellation: .untracked + ) { driver in ObjectIdentifier(driver) } + + #expect(route == .sessionDriver) + #expect(ranOn == ObjectIdentifier(harness.home)) + } + + @Test("Closing a database's entry closes its connection") + func closingTheEntryClosesItsConnection() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + + DatabaseManager.shared.sessionLanes.close(database: "app", for: harness.connection.id) + + #expect(harness.home.disconnectCallCount == 1) + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + #expect(DatabaseManager.shared.executionRoute(for: scope) == .pooled) + } + + @Test("Ending the session closes every database's connection") + func endingTheSessionClosesThemAll() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + + DatabaseManager.shared.removeSession(for: harness.connection.id) + + #expect(harness.home.disconnectCallCount == 1) + #expect(DatabaseManager.shared.sessionLanes.parkedDatabases(for: harness.connection.id).isEmpty) + } + + /// A ping that failed on the connection left behind must not reconnect the one the user moved + /// onto, which would take its transaction with it. + @Test("A reconnect for a failed check leaves a connection that replaced the checked one alone") + func reconnectIsFencedOnTheCheckedDriver() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + let logs = try #require(harness.opener.opened["logs"]) + + let outcome = await DatabaseManager.shared.performHealthMonitorReconnect( + connectionId: harness.connection.id, + failedDriver: ObjectIdentifier(harness.home) + ) + + #expect(outcome == .success) + #expect(session(harness)?.driver === logs) + #expect(logs.disconnectCallCount == 0) + } + + @Test("A parked connection that died holding a transaction is replaced and the work refused once") + func deadParkedTransactionIsReported() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + harness.home.pingError = DatabaseError.connectionFailed("server closed the connection") + harness.home.sessionTransactionStateToReturn = .inTransaction + harness.home.pingFailureForgetsTransactionState = true + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + + await #expect(throws: (any Error).self) { + try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { _ in () } + } + + let replacement = try #require(harness.opener.opened["app"]) + #expect(harness.home.disconnectCallCount == 1) + #expect(DatabaseManager.shared.sessionLanes.parkedDriver(for: harness.connection.id, database: "app") === replacement) + let ranOn = try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { driver in ObjectIdentifier(driver) } + #expect(ranOn == ObjectIdentifier(replacement)) + } + + @Test("A parked connection that died idle is replaced without failing the work") + func deadIdleParkedConnectionIsReplacedQuietly() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + harness.home.pingError = DatabaseError.connectionFailed("server closed the connection") + harness.home.sessionTransactionStateToReturn = .idle + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + + let ranOn = try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { driver in ObjectIdentifier(driver) } + + let replacement = try #require(harness.opener.opened["app"]) + #expect(ranOn == ObjectIdentifier(replacement)) + } + + /// Coming back to a database is a promotion, not a turn, so it has to make the same check: a + /// parked connection that died holding a transaction is replaced, and the next work is refused once. + @Test("Switching back to a database whose connection died holding a transaction reports it once") + func switchingBackToADeadTransactionReportsIt() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + harness.home.pingError = DatabaseError.connectionFailed("server closed the connection") + harness.home.sessionTransactionStateToReturn = .inTransaction + harness.home.pingFailureForgetsTransactionState = true + + try await DatabaseManager.shared.switchDatabase(to: "app", for: harness.connection.id, persist: false) + + let replacement = try #require(harness.opener.opened["app"]) + #expect(session(harness)?.driver === replacement) + #expect(harness.home.disconnectCallCount == 1) + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + await #expect(throws: (any Error).self) { + try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { _ in () } + } + let ranOn = try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { driver in ObjectIdentifier(driver) } + #expect(ranOn == ObjectIdentifier(replacement)) + } + + /// A check that is still pinging holds the database's turn, which is no proof the connection + /// answers, so a switch back waits for the check and takes what it settles on. + @Test("Switching back while a check of the parked connection runs promotes what the check settles on") + func switchingBackWaitsForARunningCheck() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + harness.home.pingError = DatabaseError.connectionFailed("server closed the connection") + harness.home.sessionTransactionStateToReturn = .idle + harness.home.pingDelaySeconds = 1 + let lanes = DatabaseManager.shared.sessionLanes + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + let work = Task { @MainActor in + try await DatabaseManager.shared.withScopedDriver( + scope: scope, route: .sessionDriver, cancellation: .untracked + ) { driver in ObjectIdentifier(driver) } + } + let deadline = Date().addingTimeInterval(5) + while !lanes.isVerifying(harness.home), Date() < deadline { + try await Task.sleep(for: .milliseconds(10)) + } + #expect(lanes.isVerifying(harness.home)) + + try await DatabaseManager.shared.switchDatabase(to: "app", for: harness.connection.id, persist: false) + + let replacement = try #require(harness.opener.opened["app"]) + #expect(session(harness)?.driver === replacement) + #expect(session(harness)?.status == .connected) + #expect(harness.home.disconnectCallCount == 1) + #expect(harness.opener.openCount == 2) + #expect(try await work.value == ObjectIdentifier(replacement)) + } + + @Test("A switch while the connection is being rebuilt promotes nothing") + func switchWaitsForATransportRebuild() async { + let harness = makeHarness() + defer { + MetadataConnectionPool.shared.endTransportReplacement(connectionId: harness.connection.id) + cleanUp(harness) + } + MetadataConnectionPool.shared.beginTransportReplacement(connectionId: harness.connection.id) + + await #expect(throws: (any Error).self) { + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + } + + #expect(harness.opener.openCount == 0) + #expect(session(harness)?.driver === harness.home) + } + + /// A schema switch moves the browsed connection's search path, so it has to take the same turn + /// as work on that database, or it lands between a statement's pin and the statement. + @Test("A schema switch waits for work running on the browsed database's connection") + func schemaSwitchSharesTheDatabaseTurn() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + let release = AsyncLatch() + let acquired = AsyncLatch() + let holder = Task { @MainActor in + try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess( + SessionDriverGate.Key(connectionId: harness.connection.id, database: "app") + ) { + acquired.open() + await release.wait() + } + } + await acquired.wait() + + let schemaSwitch = Task { @MainActor in + try await DatabaseManager.shared.switchSchema(to: "reporting", for: harness.connection.id) + } + for _ in 0..<10_000 where DatabaseManager.shared.sessionDriverGate.waiterCount(for: harness.connection.id) < 1 { + await Task.yield() + } + + #expect(DatabaseManager.shared.sessionDriverGate.waiterCount(for: harness.connection.id) == 1) + #expect(harness.home.switchSchemaCallCount == 0) + release.open() + try await holder.value + try await schemaSwitch.value + #expect(harness.home.switchSchemaCallCount == 1) + } + + @Test("A rebuilt transport reports each parked transaction it ended, once") + func transportRebuildReportsParkedTransactions() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + harness.home.sessionTransactionStateToReturn = .inTransaction + let scope = DatabaseScope(connectionId: harness.connection.id, database: "app", schema: nil) + + await DatabaseManager.shared.sessionLanes.closeAllNotingLostTransactions(for: harness.connection.id) + + #expect(harness.home.disconnectCallCount == 1) + #expect(DatabaseManager.shared.executionRoute(for: scope) == .sessionDriver) + await #expect(throws: (any Error).self) { + try await DatabaseManager.shared.withScopedDriver( + scope: scope, + route: DatabaseManager.shared.executionRoute(for: scope), + cancellation: .untracked + ) { _ in () } + } + #expect(DatabaseManager.shared.executionRoute(for: scope) == .pooled) + } + + @Test("Closing a database supersedes a reopen of it still in flight") + func closingSupersedesAReopen() { + let harness = makeHarness() + defer { cleanUp(harness) } + let lanes = DatabaseManager.shared.sessionLanes + let before = lanes.generation(for: harness.connection.id) + + lanes.close(database: "app", for: harness.connection.id) + + #expect(!lanes.isCurrent(before, for: harness.connection.id)) + } + + @Test("A driver that reported its connection lost is not trusted on a recent check") + func lostConnectionIsNotFresh() { + let driver = MockDatabaseDriver() + let lanes = SessionLanes(opener: { _ in driver }) + lanes.markVerified(driver) + #expect(lanes.isFresh(driver)) + + driver.hasLostConnection = true + + #expect(!lanes.isFresh(driver)) + } + + @Test("Switching database saves the schema of the database switched to") + func switchSavesTheTargetSchema() async throws { + let harness = makeHarness() + defer { cleanUp(harness) } + AppSettingsStorage.shared.saveLastSchema("app_schema", for: harness.connection.id) + harness.opener.schemaForNextOpen = "logs_schema" + + try await DatabaseManager.shared.switchDatabase(to: "logs", for: harness.connection.id, persist: false) + + #expect(AppSettingsStorage.shared.loadLastSchema(for: harness.connection.id) == "logs_schema") + } +} + +@MainActor +private final class AsyncLatch { + private var waiters: [CheckedContinuation] = [] + private var isOpen = false + + func open() { + guard !isOpen else { return } + isOpen = true + let pending = waiters + waiters = [] + for waiter in pending { + waiter.resume() + } + } + + func wait() async { + guard !isOpen else { return } + await withCheckedContinuation { waiters.append($0) } + } +} diff --git a/TableProTests/Core/Database/SwitchDatabasePooledConnectionTests.swift b/TableProTests/Core/Database/SwitchDatabasePooledConnectionTests.swift index 552b3dc535..cf04efb62f 100644 --- a/TableProTests/Core/Database/SwitchDatabasePooledConnectionTests.swift +++ b/TableProTests/Core/Database/SwitchDatabasePooledConnectionTests.swift @@ -23,7 +23,7 @@ struct SwitchDatabasePooledConnectionTests { private func registerTypeIfNeeded() { guard PluginMetadataRegistry.shared.snapshot(forRegisteredTypeId: Self.typeId) == nil else { return } let defaults = PluginMetadataSnapshot.CapabilityFlags.defaults - let capabilities = PluginMetadataSnapshot.CapabilityFlags( + var capabilities = PluginMetadataSnapshot.CapabilityFlags( supportsSchemaSwitching: true, supportsImport: defaults.supportsImport, supportsExport: defaults.supportsExport, @@ -36,6 +36,9 @@ struct SwitchDatabasePooledConnectionTests { requiresReconnectForDatabaseSwitch: true, supportsDropDatabase: defaults.supportsDropDatabase ) + /// No second connection, so a switch still takes the reconnect this suite is about rather + /// than opening a connection of its own for the database. + capabilities.supportsConnectionPooling = false let snapshot = PluginMetadataSnapshot( displayName: Self.typeId, iconName: "cylinder", defaultPort: 1_234, requiresAuthentication: true, supportsForeignKeys: true, supportsSchemaEditing: true, diff --git a/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift b/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift index a68a83b505..9da0c78b51 100644 --- a/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift +++ b/TableProTests/Core/Database/SwitchDatabaseReconnectFailureTests.swift @@ -36,6 +36,9 @@ struct SwitchDatabaseReconnectFailureTests { requiresReconnectForDatabaseSwitch: true, supportsDropDatabase: capabilities.supportsDropDatabase ) + /// No second connection, so a switch still takes the reconnect this suite is about rather + /// than opening a connection of its own for the database. + capabilities.supportsConnectionPooling = false let snapshot = PluginMetadataSnapshot( displayName: Self.typeId, iconName: "cylinder", defaultPort: 1_234, requiresAuthentication: true, supportsForeignKeys: true, supportsSchemaEditing: true, diff --git a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift index 1a120f8a37..c7661521e8 100644 --- a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift +++ b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift @@ -150,4 +150,33 @@ struct ImportDataSinkAdapterMappingTests { ["name": .text("Grace")], ]) } + + /// A session connection can already hold the user's own transaction, as a database kept open + /// for its rail entry does. Opening one there joined it, and the import's COMMIT then committed + /// the user's pending work with it. + @Test("An import into a session holding the user's transaction neither opens nor commits one") + func importJoinsTheUsersTransaction() async throws { + let driver = MockDatabaseDriver() + driver.sessionTransactionStateToReturn = .inTransaction + let sink = ImportDataSinkAdapter(driver: driver, databaseType: .mysql, targetTable: "people") + + try await sink.beginTransaction() + try await sink.commitTransaction() + + #expect(driver.beginTransactionCallCount == 0) + #expect(driver.commitTransactionCallCount == 0) + } + + @Test("An import into an idle session opens and commits its own transaction") + func importOwnsItsTransactionOnAnIdleSession() async throws { + let driver = MockDatabaseDriver() + driver.sessionTransactionStateToReturn = .idle + let sink = ImportDataSinkAdapter(driver: driver, databaseType: .mysql, targetTable: "people") + + try await sink.beginTransaction() + try await sink.commitTransaction() + + #expect(driver.beginTransactionCallCount == 1) + #expect(driver.commitTransactionCallCount == 1) + } } diff --git a/TableProTests/Plugins/LibPQKeepaliveTests.swift b/TableProTests/Plugins/LibPQKeepaliveTests.swift new file mode 100644 index 0000000000..5e80f61440 --- /dev/null +++ b/TableProTests/Plugins/LibPQKeepaliveTests.swift @@ -0,0 +1,107 @@ +// +// LibPQKeepaliveTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +struct LibPQKeepaliveTests { + private static let pluginDirectory: URL = { + var directory = URL(fileURLWithPath: #filePath) + for _ in 0..<3 { directory.deleteLastPathComponent() } + return directory + .appendingPathComponent("Plugins") + .appendingPathComponent("PostgreSQLDriverPlugin") + }() + + private func build(options: String? = nil) -> String { + LibPQConnectionString.build( + host: "db.example.com", + port: 5_432, + user: "postgres", + password: "hunter2", + database: "app", + sslConfig: SSLConfiguration(), + options: options, + applicationName: "TablePro", + connectTimeoutSeconds: 10 + ) + } + + /// Reads the `keyword='value'` pairs the way libpq does, so a keyword spelled inside a quoted + /// value, such as the connection options, is not counted as one of the connection's own. + private func keywords(in conninfo: String) throws -> [String: [String]] { + let pair = try NSRegularExpression(pattern: #"(\w+)='((?:[^'\\]|\\.)*)'"#) + let text = conninfo as NSString + var found: [String: [String]] = [:] + for match in pair.matches(in: conninfo, range: NSRange(location: 0, length: text.length)) { + found[text.substring(with: match.range(at: 1)), default: []] + .append(text.substring(with: match.range(at: 2))) + } + return found + } + + @Test("Every connection asks for a keepalive after 60 s idle, then 3 probes 10 s apart") + func sendsKeepalives() throws { + let found = try keywords(in: build()) + + #expect(found["keepalives"] == ["1"]) + #expect(found["keepalives_idle"] == ["60"]) + #expect(found["keepalives_interval"] == ["10"]) + #expect(found["keepalives_count"] == ["3"]) + } + + /// Measured on macOS with the libpq the app ships: the `keepalives_*` values reach the socket, + /// and `tcp_user_timeout` leaves `TCP_RXT_CONNDROPTIME` at 0. + @Test("tcp_user_timeout is not sent, because macOS ignores it") + func omitsUserTimeout() throws { + #expect(try keywords(in: build())["tcp_user_timeout"] == nil) + } + + /// The Connection Options field becomes libpq's `options` keyword, which the server reads as + /// its own settings, so a user cannot set the client's keepalives there. The server's + /// `tcp_keepalives_*` settings govern the server's end of the same socket and still apply. + @Test( + "Server keepalive settings in the connection options reach the server beside the client's own", + arguments: ["-c tcp_keepalives_idle=30", "--tcp-keepalives-interval=5 -c tcp_keepalives_count=2"] + ) + func serverKeepalivesStayInOptions(options: String) throws { + let found = try keywords(in: build(options: options)) + + #expect(found["options"] == [options]) + #expect(found["keepalives_idle"] == ["60"]) + #expect(found["keepalives_interval"] == ["10"]) + #expect(found["keepalives_count"] == ["3"]) + } + + @Test("The connection options stay the last keyword after the keepalives are added") + func optionsStayLast() { + #expect(build(options: "-c search_path=app").hasSuffix(" options='-c search_path=app'")) + } + + /// The keepalives live in the conninfo builder, so they reach every PostgreSQL, Redshift, + /// CockroachDB and PGlite connection only while that builder is the one way the plugin dials. + @Test("The plugin opens libpq connections in one place, from the built conninfo") + func oneConnectPath() throws { + let sources: [(name: String, text: String)] = try FileManager.default + .contentsOfDirectory(at: Self.pluginDirectory, includingPropertiesForKeys: nil) + .filter { $0.pathExtension == "swift" } + .map { (name: $0.lastPathComponent, text: try String(contentsOf: $0, encoding: .utf8)) } + #expect(sources.contains { $0.name == "LibPQConnectionString.swift" }) + + let otherDialers = ["PQconnectdb", "PQconnectdbParams", "PQconnectStartParams", "PQsetdbLogin"] + let offenders = sources.flatMap { source in + otherDialers.filter { source.text.contains("\($0)(") }.map { "\(source.name): \($0)" } + } + #expect(offenders.isEmpty, "\(offenders)") + + let starts = sources.filter { $0.text.contains("PQconnectStart(") } + #expect(starts.map { $0.name } == ["LibPQPluginConnection.swift"]) + let connection = try #require(starts.first?.text) + #expect(connection.components(separatedBy: "PQconnectStart(").count == 2) + #expect(connection.contains("connectionString.withCString({ PQconnectStart($0) })")) + #expect(connection.contains("private var connectionString: String {\n LibPQConnectionString.build(")) + } +} diff --git a/TableProTests/Plugins/LibPQSessionCheckTests.swift b/TableProTests/Plugins/LibPQSessionCheckTests.swift new file mode 100644 index 0000000000..9054a1779a --- /dev/null +++ b/TableProTests/Plugins/LibPQSessionCheckTests.swift @@ -0,0 +1,115 @@ +// +// LibPQSessionCheckTests.swift +// TableProTests +// + +import Foundation +import TableProPluginKit +import Testing + +struct LibPQSessionCheckTests { + private static func pluginSource(_ name: String) throws -> String { + var directory = URL(fileURLWithPath: #filePath) + for _ in 0..<3 { directory.deleteLastPathComponent() } + return try String( + contentsOf: directory + .appendingPathComponent("Plugins") + .appendingPathComponent("PostgreSQLDriverPlugin") + .appendingPathComponent(name), + encoding: .utf8 + ) + } + + /// The body of the first `signature` in `source`, up to the brace that closes a member indented + /// four spaces, which is how every member of these two classes is laid out. + private static func body(of signature: String, in source: String) throws -> String { + let start = try #require(source.range(of: signature)) + let end = try #require(source.range(of: "\n }\n", range: start.upperBound.. LibPQPluginError { + LibPQPluginError(message: message, sqlState: sqlState, detail: nil) + } + + @Test("Inside a transaction block, open or aborted, the check sends no statement") + func noStatementInsideATransaction() { + #expect(!LibPQSessionCheck.sendsStatement(in: .inTransaction)) + #expect(!LibPQSessionCheck.sendsStatement(in: .inError)) + } + + @Test("Outside a transaction block, or when the state is not known, the check makes a round trip") + func roundTripOtherwise() { + #expect(LibPQSessionCheck.sendsStatement(in: .idle)) + #expect(LibPQSessionCheck.sendsStatement(in: .active)) + #expect(LibPQSessionCheck.sendsStatement(in: .unknown)) + } + + /// Measured on PostgreSQL 17.11: after a failed statement inside `BEGIN`, `SELECT 1` answers + /// `25P02` on a session that is still there. + @Test("A statement refused inside an aborted transaction is the server's answer") + func abortedTransactionAnswers() { + let refused = Self.serverError( + "25P02", + "ERROR: current transaction is aborted, commands ignored until end of transaction block" + ) + #expect(LibPQSessionCheck.refusalState(of: refused) == "25P02") + } + + @Test("Any server error with a SQLSTATE is an answer", arguments: ["57014", "53200", "42501", "XX000"]) + func serverErrorsAnswer(sqlState: String) { + #expect(LibPQSessionCheck.refusalState(of: Self.serverError(sqlState)) == sqlState) + } + + @Test("A lost session is not an answer, even with the server's SQLSTATE on it") + func lostSessionIsNotAnAnswer() { + let fatal = Self.serverError("57P01", "FATAL: terminating connection due to administrator command") + let losses: [LibPQConnectionLoss] = [ + .afterSending, + .beforeSending(transactionMayBeOpen: true), + .beforeSending(transactionMayBeOpen: false) + ] + + for loss in losses { + let lost = LibPQConnectionLostError(loss: loss, underlying: fatal) + #expect(LibPQSessionCheck.refusalState(of: lost) == nil, "\(loss)") + } + } + + @Test("libpq's own failures and a cancellation are not an answer") + func clientFailuresAreNotAnAnswer() { + #expect(LibPQSessionCheck.refusalState(of: LibPQPluginError.notConnected) == nil) + #expect(LibPQSessionCheck.refusalState(of: LibPQPluginError.connectionTimedOut) == nil) + #expect(LibPQSessionCheck.refusalState(of: Self.serverError(nil, "server closed the connection unexpectedly")) == nil) + #expect(LibPQSessionCheck.refusalState(of: Self.serverError("")) == nil) + #expect(LibPQSessionCheck.refusalState(of: CancellationError()) == nil) + } + + /// `LibPQPluginConnection` imports CLibPQ, which this target cannot, so the wiring is checked in + /// the source: read the socket before anything else, send nothing inside a transaction block, + /// and take a refusal as an answer only while libpq still reports the connection OK. + @Test("The plugin's ping reads the socket first and accepts a refusal only on a live connection") + func pingIsWiredThroughTheCheck() throws { + let ping = try Self.body( + of: "func ping() async throws {", + in: Self.pluginSource("LibPQPluginConnection.swift") + ) + let socketRead = try #require(ping.range(of: "sessionEndedBeforeSending(conn)")) + let decision = try #require(ping.range(of: "LibPQSessionCheck.sendsStatement(in: transactionStateOnQueue())")) + let statement = try #require(ping.range(of: "executeQuerySync(LibPQSessionCheck.statement)")) + let liveCheck = try #require(ping.range(of: "PQstatus(conn) == CONNECTION_OK")) + let refusal = try #require(ping.range(of: "LibPQSessionCheck.refusalState(of: error)")) + + #expect(socketRead.upperBound < decision.lowerBound) + #expect(decision.upperBound < statement.lowerBound) + #expect(statement.upperBound < liveCheck.lowerBound) + #expect(liveCheck.upperBound < refusal.lowerBound) + + let corePing = try Self.body( + of: "func ping() async throws {", + in: Self.pluginSource("LibPQDriverCore.swift") + ) + #expect(corePing.contains("try await pqConn.ping()")) + #expect(!corePing.contains("executeQuery")) + } +} diff --git a/TableProTests/Views/SwitchSchemaTests.swift b/TableProTests/Views/SwitchSchemaTests.swift index 1bd3c069ee..2684f01830 100644 --- a/TableProTests/Views/SwitchSchemaTests.swift +++ b/TableProTests/Views/SwitchSchemaTests.swift @@ -59,8 +59,10 @@ struct SwitchSchemaTests { let acquired = SchemaSwitchLatch() let release = SchemaSwitchLatch() + /// PostgreSQL takes one turn per database, so the turn held is the browsed database's. + let gateKey = DatabaseManager.shared.browsedGateKey(for: connection.id) let holder = Task { @MainActor in - try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess(connection.id) { + try await DatabaseManager.shared.sessionDriverGate.withExclusiveAccess(gateKey) { acquired.open() await release.wait() } diff --git a/docs/databases/cockroachdb.mdx b/docs/databases/cockroachdb.mdx index 99ab3cd2df..f3339d11bb 100644 --- a/docs/databases/cockroachdb.mdx +++ b/docs/databases/cockroachdb.mdx @@ -45,7 +45,7 @@ Table and view DDL comes from CockroachDB's own `SHOW CREATE`, so the DDL tab sh `EXPLAIN` and `EXPLAIN ANALYZE` render as a visual plan tree. CockroachDB returns text plans rather than JSON, and the text is parsed into the tree. See [EXPLAIN Visualization](/features/explain-visualization). -Changing database means reconnecting, as on PostgreSQL. A tab bound to a database other than the connection's active one runs on a separate connection for it, sharing no temp tables, session variables, or open transaction with the query editor. See [Cross-database tabs](/databases/postgresql#cross-database-tabs). +As on PostgreSQL, each database browsed from the connections strip keeps a connection of its own, so moving between databases does not reconnect, and a tab runs on its database's connection. See [Cross-database tabs](/databases/postgresql#cross-database-tabs). ## Limitations diff --git a/docs/databases/postgresql.mdx b/docs/databases/postgresql.mdx index 5c2ffe13cf..def1b6c2b7 100644 --- a/docs/databases/postgresql.mdx +++ b/docs/databases/postgresql.mdx @@ -5,7 +5,7 @@ description: Connect to PostgreSQL 9.1 and later with the libpq driver, includin import DeclaredColumnType from "/snippets/declared-column-type.mdx"; -Unlike MySQL, PostgreSQL will not connect without a **Database**, and it changes database only by reconnecting. Everything else on the form is ordinary. The libpq driver ships inside the app and also serves [Amazon Redshift](/databases/redshift), [CockroachDB](/databases/cockroachdb), and [PGlite](/databases/pglite). +Unlike MySQL, PostgreSQL will not connect without a **Database**, and a connection to the server serves that one database. Everything else on the form is ordinary. The libpq driver ships inside the app and also serves [Amazon Redshift](/databases/redshift), [CockroachDB](/databases/cockroachdb), and [PGlite](/databases/pglite). ## Connection settings @@ -122,11 +122,13 @@ A `CREATE INDEX CONCURRENTLY` or `REINDEX CONCURRENTLY` that fails or is cancell ## Cross-database tabs -PostgreSQL has no in-place `USE`, so a tab bound to a database other than the connection's active one runs on a second connection opened for that database. It shares no temp tables, session variables, or open transaction with the query editor on the main connection: keep a multi-statement transaction or a `CREATE TEMP TABLE` on tabs bound to one database. Binding itself is on [Tabs](/features/tabs#where-a-tab-points). +PostgreSQL has no in-place `USE`, so every database browsed from the [connections strip](/features/workspace-rail) gets a server connection of its own. Moving between two database entries switches between those connections without reconnecting, and each keeps its open transaction, temp tables, and `SET` values until its entry is closed or the connection ends. A tab runs on the connection of the database it is bound to. + +A tab bound to a database that has no entry runs on a [metadata connection](/connections/connection-form#connection-health) instead. That one is shared with the Structure tab and closes after 10 minutes idle, so open the database from the strip before starting a transaction or a `CREATE TEMP TABLE` there. A database connection that died while idle is reopened on its next use; if it held a transaction, that statement is refused once with a message saying the server rolled it back. Binding itself is on [Tabs](/features/tabs#where-a-tab-points). ## Sessions on the server -In `pg_stat_activity`, the connection your query editor uses reports `application_name` as `TablePro`. The [metadata connections](/connections/connection-form#connection-health) that read the sidebar and the Structure tab, and the ones a cross-database tab runs on, report `TablePro Metadata`. To name them yourself, put `-c application_name=reporting` in **Connection Options**: every connection then uses that name. +In `pg_stat_activity`, each database's own connection reports `application_name` as `TablePro`, one per database entry in the strip. The [metadata connections](/connections/connection-form#connection-health) that read the sidebar and the Structure tab, and the ones a tab on a database without an entry runs on, report `TablePro Metadata`. To name them yourself, put `-c application_name=reporting` in **Connection Options**: every connection then uses that name. ## Text encoding @@ -143,7 +145,7 @@ New connections default to **Preferred** (libpq `sslmode=prefer`): TLS first, pl ## Limitations - Columns cannot be reordered. The structure editor adds, renames, retypes, and drops; changing the order of existing columns means recreating the table. -- A cross-database tab cannot share session state with the main connection. Statements that depend on a temp table or an open transaction have to run on one database. +- Session state belongs to one database. A temp table or an open transaction made on `app` is not visible from a tab on `logs`, and closing the `app` entry ends it. - Backup and restore need `pg_dump` and `pg_restore` on your Mac. Neither is bundled; install them with Homebrew. `pg_dump` 15 and later refuse PostgreSQL 9.1, so back one up with `pg_dump` 14 or earlier. - Before PostgreSQL 12, the [`run_maintenance`](/external-api/mcp-tools) tool's `REINDEX` with no `table` runs without `CONCURRENTLY` and rebuilds the system catalog indexes as well. While an index rebuilds, writes to its table wait, and so do reads that use that index. Run it when the database is quiet. diff --git a/docs/features/tabs.mdx b/docs/features/tabs.mdx index 2eade1bd08..9c1253c4c8 100644 --- a/docs/features/tabs.mdx +++ b/docs/features/tabs.mdx @@ -125,7 +125,7 @@ A tab is bound to one connection, database, and schema, and every query, refresh The centred toolbar control names the frontmost tab's database, and clicking it switches to another. That is how you tell two tabs on one connection apart. On engines with schemas, **Database > Schema** names the one in use and switches it. -PostgreSQL, Redshift, and CockroachDB change database only by reconnecting, so a tab bound to a second database runs on its own connection: no shared temp tables, session variables, or open transaction. See [PostgreSQL](/databases/postgresql#cross-database-tabs). +PostgreSQL, Redshift, and CockroachDB keep one connection per database in the connections strip, and a tab runs on the connection of the database it is bound to. Temp tables, session variables, and an open transaction stay with that database. See [PostgreSQL](/databases/postgresql#cross-database-tabs). ## What survives a restart diff --git a/docs/features/workspace-rail.mdx b/docs/features/workspace-rail.mdx index 35fc64d1b4..c9725579d8 100644 --- a/docs/features/workspace-rail.mdx +++ b/docs/features/workspace-rail.mdx @@ -46,7 +46,7 @@ Closing an entry's last tab leaves the entry where it is: tabs and entries close Between two connections, the window switches connection and returns you to the tab you last used there; between two databases of one connection, the sidebar moves and the tabs stay. An entry with nothing open moves the sidebar only, and clicking the entry you are in does nothing. No tab is ever closed or retargeted by a switch: each one keeps querying the database it was opened against. -Coming back to a connection reloads nothing it already had. The tab in front keeps its rows, scroll position and unsaved edits, and the object list and [autocomplete](/features/autocomplete#my-new-table-does-not-complete) keep what they loaded. To pick up a table created from another client, press `Cmd+R`. +On PostgreSQL, Redshift, and CockroachDB every database entry keeps its own server connection, so moving between two entries reconnects nothing and leaves each database's open transaction and temp tables where they were. Closing an entry closes its connection. The strip takes the keyboard too. Click into it, then use the arrow keys to move the highlight. Type the start of a connection, database, or schema name to jump to it, and press `Return` to open the entry.