`AuthenticationSessionsMiddleware` should use a connection pool about auth HOT 10 CLOSED

@MrMage see here: https://github.com/vapor/fluent-postgresql/blob/master/Sources/FluentPostgresDriver/Postgres%2BFluent.swift#L35

ConnectionPool conforms to Fluent.Database using withConnection(_:).

from auth.

Yeah, this is unfortunately a big problem with Vapor 3's design. See here for additional discussion: vapor/vapor#1711

Vapor 4's design is exactly what you described: anything that needs connections, whether it be a middleware or a controller will get a pool. Each query to the DB will pop a connection from that pool for the lifetime of the query. This will allow a max connection pool size of 1 to work without causing deadlocks.

As for porting this fix back to Vapor 3, I'm not sure it's worth the potential issues it could cause. I see this as more of a fundamental design flaw than a bug.

from auth.

See also: vapor/fluent#564 (comment)

from auth.

Vapor 4's design is exactly what you described: anything that needs connections, whether it be a middleware or a controller will get a pool. Each query to the DB will pop a connection from that pool for the lifetime of the query. This will allow a max connection pool size of 1 to work without causing deadlocks.

That sounds great! Would you mind providing a pointer to how this is going to be implemented (or is implemented) in Fluent 4? In particular, how does the pool know when the query is finished? For something like db.withPooledConnection { connection in query(on: connection) }, this is obvious, but how would

let db = try container.connectionPool(for: .sqlite)
router.get("work") { req -> String in
    _ = Todo.query(on: db).all().map { todos in
        print(todos)
    }
    return "done"
}

work with regards to detecting when the query is done?

As for porting this fix back to Vapor 3, I'm not sure it's worth the potential issues it could cause. I see this as more of a fundamental design flaw than a bug.

Agreed. I guess we can close this issue, but I'd still like to have a look at the Fluent 4 pool implementation :-)

from auth.

ConnectionPool conforms to Fluent.Database using withConnection(_:).

Looks great! I guess in theory the connection could even be released before onOutput is called; is that the case?

from auth.

Theoretically yes, but in reality NIOMySQL and NIOPostgres don't complete their return futures until they have passed back all rows to the user.

from auth.

So… for those of us are experiencing this and who are building something where we anticipate heavy usage in the near future, we should opt in to beta test Vapor 4?

from auth.

@mxcl Depends on your database setup mostly. If you can have a pool size per loop greater than the potential deadlock minimum, then you're ok. Otherwise you should either generate a database connection manually (instead of passing in Request as the DatabaseConnectable object, since that uses request caching) or try out Vapor 4 (Forewarning, I have no idea what the state of the alphas/betas are, I don't think they're even out yet)

from auth.

I appreciate the advice, thank you.

from auth.

No problem, let me know if I can help in any way!

from auth.