fluent-kit icon indicating copy to clipboard operation
fluent-kit copied to clipboard

Retry failed query

Open TofPlay opened this issue 6 years ago • 6 comments

On my application I have a very basic implementation like:

final public class MyController: RouteCollection {

    public func boot(router: Router) throws {
        router.get("myapi", use: myapi)
    }

     private func myapi(_ request: Request) throws -> Future<[MyData]>  {
        return try MyData.query(on: request).all()
    }

}

On macOS everything is going well On Linux from time to time when I try to use the request object I have the error:

 read(descriptor:pointer:size:) failed: Connection reset by peer (errno: 104)

I can not even catch the error and make a retry because the connection with the database at the object request is closed I searched but I did not find a way to reset the connection with the database from the request object

Suggestion: This error Connection reset by peer should be supported at fluent level in the form of a auto-retry mechanism. An error the connection was closed because of a network error should be throw only if the auto-retry can not restore the connection with the database.

TofPlay avatar Feb 25 '19 14:02 TofPlay

I think there's a v3 bug involved, hidden between the lines. I've discussed this issue with Tof in private, and it seems the Request is caching the dead connection, with no way to eject it.

Consider a route handler that uses SomeModel.query(on: req). If the database connection dies, retrying the query in a catchFlatMap will always fail, as the dead connection is returned for each and every query attempt.

We've found that the only way to evade this at the moment is to have the catchFlatMap manually reconnects with other methods, like withNewConnection or withPooledConnection.

This is obviously very frustrating, and forces the user to work around the framework again in order to manually fix something that shouldn't be a concern in the first place.

vzsg avatar Feb 28 '19 15:02 vzsg

We've encountered almost the same problem (also with PostgreSQL). But in our case it's happening on macOS. It's a rather simple Vapor application that (after some time) starts failing queries.

In our case, there aren't too many request coming in. So a request comes in and opens a DatabaseConnectionPool with one connection in it. This connection is then idling for quite some time (sometimes for a complete day). I assume that the database is closing the connection after some time, but I've found no code in Fluent (or the PostgreSQL driver) that handles connections being closed by the remote. Like @vzsg mentioned there seems to be no way to get rid of those connections. Their isClosed property still reports false (since they weren't closed by calling close()) and therefore they're still returned by the pool.

We do have multiple Vapor applications running (that use PostgreSQL as database) on our Linux server without any issues. The difference between them and the application running on our macOS server now is that those on Linux have the database on the same machine. The macOS one connects to the database on the Linux server. So I think the connection drops become more frequent when there's a "longer route" from the application to the database.

ffried avatar Apr 15 '19 13:04 ffried

I've implemented a little workaround to get working connections. It's not perfect but seems to do the job in our case.

private let connectionClosedErrNos: Set<Int32> = [
    ECONNRESET, // Connection reset by peer
    EHOSTUNREACH, // No route to host
    EPIPE, // Broken pipe
]

extension DatabaseConnectable {
    private func log(_ msg: String, file: String = #file, function: String = #function, line: UInt = #line, column: UInt = #column) {
        (try? (self as? Container)?.log().info(msg, file: file, function: function, line: line, column: column)) ?? print(msg)
    }

    private func releaseCachedConnections() throws {
        // This will make sure the container does not cache the connection and return it even though it has been closed.
        switch self {
        case let request as Request: try request.privateContainer.releaseCachedConnections()
        case let response as Response: try response.privateContainer.releaseCachedConnections()
        case let container as Container: try container.releaseCachedConnections()
        default: log("Cannot release cached connections of \(self)! Missing conformance to \(Container.self)!")
        }
    }

    func workingDatabaseConnection<Database>(to database: DatabaseIdentifier<Database>?) -> Future<Database.Connection>
        where Database.Connection: SQLConnectable
    {
        return databaseConnection(to: database).flatMap { conn in
            conn.raw("select 1;").run()
                .transform(to: conn)
                .catchFlatMap {
                    switch $0 {
                    case let ioError as IOError where connectionClosedErrNos.contains(ioError.errnoCode):
                        self.log("Closing dead connection: \(conn)!\nWill retry to get new connection.")
                        conn.close()
                    case let pgError as PostgreSQLError where pgError.identifier == "closed":
                        self.log("Found closed connection: \(conn)!\nWill retry to get new connection.")
                    default:
                        self.log("Encountered unhandled error while running validation query on \(conn): \($0)! Will re-throw error!")
                        throw $0
                    }
                    try self.releaseCachedConnections()
                    return self.workingDatabaseConnection(to: database)
            }
        }
    }
}

extension Model where Database: QuerySupporting, Database.Connection: SQLConnectable {
    static func safeQuery(on connectable: DatabaseConnectable) -> Future<QueryBuilder<Database, Self>> {
        return connectable.workingDatabaseConnection(to: Self.defaultDatabase).map { query(on: $0) }
    }

    static func safeFind(_ id: ID, on connectable: DatabaseConnectable) -> Future<Self?> {
        return connectable.workingDatabaseConnection(to: Self.defaultDatabase).flatMap { find(id, on: $0) }
    }

    func safeSave(on connectable: DatabaseConnectable) -> Future<Self> {
        return connectable.workingDatabaseConnection(to: Self.defaultDatabase).flatMap(save(on:))
    }

    /* add more convenience implementations if necessary */
}

ffried avatar Apr 15 '19 14:04 ffried

Hello @ffried do you have a workaround for Vapor 4, by any chance?

HashedViking avatar Dec 01 '20 12:12 HashedViking

@HashedViking To be honest, I was too lazy to port the fix to Vapor 4. Instead I've added this snippet to my configure.swift:

// Schedule DB reinitialization every night at 3:30...
let dateComponents = DateComponents(hour: 3, minute: 30)
let interval = Calendar.current.nextDate(after: Date(), matching: dateComponents, matchingPolicy: .nextTime)!.timeIntervalSinceNow
let nsecPerSec = TimeAmount.seconds(1).nanoseconds
let seconds = TimeAmount.seconds(.init(interval))
let nanoseconds = .nanoseconds(.init(interval * TimeInterval(nsecPerSec))) - seconds
app.eventLoopGroup.next().scheduleRepeatedTask(initialDelay: seconds + nanoseconds, delay: .hours(24), notifying: nil) { _ in
    app.logger.info("Reinitializing DB...")
    app.databases.reinitialize(.psql)
}

ffried avatar Dec 01 '20 13:12 ffried

Thank you, @ffried, you helped me to come to a solution explained in this issue

HashedViking avatar Dec 01 '20 19:12 HashedViking