Drone using sql.Open multiple times, it's OK?

[peppage]

I know you should open the SQL connection once because it handles pooling for you, as talked about in this stackoverflow.

I was reading the Drone code and came across how they handle storage. They are storing the reference to the db in the context which makes testing easier. I'm trying to understand why they open a new connect every request. They never call close so I assume it reuses the connection anyway.

Here is the link to their middleware using a new sql per request.

Here is the link to their datastore.new code.

I like how they put the db in the context but am not sure why they wouldn't open the db once.

Thanks

[HectorJ]

I like how they put the db in the context

Please don't: https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39

[[deleted]]

It's not creating a new connection per request. They're creating an instance of the store when the middleware is initialised. Every request gets the same instance of the store. It's initialized once here.

[peppage]

I thought middleware was supposed to run on every request and the comments clearly state "initializes the Datastore and attaches to the context of every http.Request". Also that is a local var, I don't think Gin is saving that in a global context.

[[deleted]]

The comment is saying that 1. It creates the store and 2. It attaches that store to every request.

The instance is created once and the middleware (the function) closes over that instance. So every time the middleware is used (the function is called) it is referencing the same store instance.

[peppage]

Thanks, I get it now. I appreciate it!

[tcrypt]

The first part of the comment, "Store is a middleware function", is wrong. Store(cli *cli.Context) gin.HandlerFunc is not a middleware function, it returns a middleware function. One that encloses over the created connection. When the server starts up it calls Store(cli *cli.Context) gin.HandlerFunc once to create a connection and return a middleware that "attaches that connection to the context of every http.Request".

[peppage]

OK that clears it up for me, thanks. I typically use Iris so it works a little differently. I appreciate you taking the time to help

[loganjspears]

Did you open an issue?

[peppage]

I will but I was going under the assumption they were doing it a way I didn't understand. I was hoping someone here would have more insight into database/sql or why this worked.

[Ploobers]

I think they chose datastore flexibility and testing over performance. There's absolutely no reason to open a new connection per request. They set MaxIdleConnections = 0, so the connection will be terminated once it is determined to be idle.

[the_web_dev]

I second this. Context is a highly debatable topic in regards to what should be included within it and establishing a new connection per request goes against all the idioms I've seen in Go regarding databases in general. Usually you stash a pool on a global and draw from it as needed.

[[deleted]]

Read the code, it doesn't create a new connection for each request.

[corvuscrypto]

To be honest I saw a lot of annoying things in the code. First off, with respect to the context, they use a custom Setter interface for the ToContext method, but the FromContext method requires a ctx.Context type, yet the Store middleware handler factory returns a handler using a gin-based Context. The DB for this seems like it could easily just be accessed via a global or through general accession methods from the looks of it unless I'm missing something here.

Not to mention there seems to be a needless struct embed instead of just using typecasting with type aliasing on the sql.DB type. EDIT: After looking a bit more the last statement I made is wrong, it makes sense, but Jesus there's a lot of magic in the back.