Expose Conn - Request for Google App Engine

[pjebs]

This is a very important feature request:

Can you please expose the Conn interface.

When we call sql.Open(...) it returns a sql.DB struct. This is part of standard library so not much we can do about that.

It is important however to get the underlying Conn for the sql.DB. This can be done via this package (since it is also what sets it via custom dialer)

URGENT FEATURE REQUEST

go-sql-driver/mysql creates some kind of function that looks like this:

func RegisterDialCallback(addr string, r http.request, conn net.Conn) (net.Conn) {

}

When the user calls sql.Open(...), then the driver calls registerDialCallback( ) and uses the returned *net.Conn

[pjebs]

The reason is, in order to use Connection pooling with Google app engine, we need to do something like this as per #466:

import (
        "database/sql"
        "github.com/go-sql-driver/mysql"
        "google.golang.org/appengine"
        "google.golang.org/appengine/socket"
)

dial := func(addr string) (net.Conn, error) {
        return socket.Dial(appengine.NewContext(r), "tcp", addr)
}

mysql.RegisterDial("external", dial)

mysql.RegisterDialCallback(addr string, r http.request, conn net.Conn) {
        if addr == "external" {
                conn.(socket.Conn).SetContext(appengine.NewContext(r))
        }
        return conn
}

What do we do here:

db, _ := sql.Open("mysql", "id:password@external(your-amazonaws-uri.com:3306)/dbname")

How do we ensure db will have RegisterDialCallback already called?

[pjebs]

Basically we need a way so that every sql.DB returned by sql.Open() will use a Conn that has conn.(socket.Conn).SetContext(...) set using the current http.Request.

There are some issues with above code when RegisterDialCallback is called and its associated sql.DB but there needs to be another workaround.

Right now, Google App Engine users must either use CloudSql OR use this driver without connection pooling on an non-cloudsql database.

[pjebs]

Perhaps, the driver provides a function where once sql.Open() returns the sql.DB, then you can ask the driver to return its associated Conn and then run the RegisterDialCallback

[pjebs]

@julienschmidt What do you think of a RegisterDialCallback function?

[arnehormann]

Where does the http.request come from in your callback? Who calls that from where? We don't have an http connection anywhere, database/sql doesn't, either.
Also, please drop the "urgent". It may just be a fitting qualifier for you, but this is just noise for me. Annoying noise, to be honest.

[pjebs]

@arnehormann

Given that I assume many people use GAE (case in point: #484), and it is not unreasonable that some of those people would like to depart from cloudsql, and given that database/sql + this package makes a big deal of connection pooling, I just thought the "Urgent" was a reasonable qualifier.

According to this: https://groups.google.com/forum/#!msg/google-appengine-go/FtWll9cYjmo/YoEZSJk_CgAJ there is a "proof-of-concept" workaround listed here:

// global variable
var conn *socket.Conn

// register a custom dial function
gsd.RegisterDial("aesocket", func(addr string) (net.Conn, error) {
    var err error
    conn, err = socket.Dial(c, "tcp", addr) //Note: c was derived somewhere in current request cycle using appengine.NewContext(r)
    return conn, err
})

// before every db call, set the new context
conn.SetContext(c)

The code above basically bypasses using database/sql connection pooling which defeats the purpose.

Now regarding you points:

I don't know the answer to those questions. I was listing the example code to highlight the pitfalls associated with getting a http.request to RegisterDialCallback and the dial func.

The dial func and mysql.RegisterDial("external", dial) can be easily set at the beginning of the request cycle.
But I don't know how to get RegisterDialCallback to be called even with a http.Request so that any queries that databasea.Open provides will have the setContext properly set.

I simply don't know how to do it - but I definitely think it will add value to this package given (I believe) how many people use GAE and how many would probably like to use an external database (with connection pooling) but can't due to this limitation.

In GAE, everything revolves around the "Request" and the conversion of that request to a Context via appengine.NewContext(r)

[methane]

It feel the proposed API is too dirty to keep backward compatibility forever, only for
"GAE classic + external MySQL" stack.

I would like to database/sql supports context.
Until then, could you create your fork for the stack?

[arnehormann]

@pjebs it may be urgent, but I'm not interested in anything but the technical part of the issue. If you maintain projects I like and use and you write it's for one of them, I might just feel more eager to help. If you have a track record of answering to issues and providing support here, I'd be a lot more eager to help. But afaik, nobody who does active development here is getting payed for support or maintenance; being pressured and nudged is off-putting.

If you propose a new api, it doesn't help if it cannot be used. In this case, we'd need the http connection. But if this is called from Open, that connection has to come from somewhere. And I don't know any code with access to it.

As is, your example can reasonable be shortened to

var httpConn func() *http.Request  = <left to reader>

dial := func(addr string) (net.Conn, error) {
        conn, err := socket.Dial(appengine.NewContext(httpConn()), "tcp", addr)
        conn.SetContext(appengine.NewContext(httpConn())) // redundant
        return conn, err
}

mysql.RegisterDial("external", dial)

because Open calls the dial function - if the callback would be called from there, the existing context would only be overridden. And the <left to reader> part is impossible to solve. Thinking if and how this could work takes time and is frustrating, so it's better to tell about the problem than to offer untested ideas for solutions which cannot work.

[pjebs]

I'm going to attempt to create a wrapper around "database/sql" that will solve this problem by using an external pool.

Is there a change that you can place a RW mutex around the dials map: https://github.com/go-sql-driver/mysql/blob/master/driver.go#L33 ?

[pjebs]

I attempted to work around database/sql's connection pooling by creating a wrapper since otherwise it requires changes to the mysql driver. My attempt Q&D work-around can be found here: https://github.com/pjebs/GAE-Toolkit-Go/tree/master/sql

If anyone wants to help go ahead.

[arnehormann]

@pjebs How would a mutex help? This is only written to from Register (usually called from init) and read from in Open. When would this clash, how do you want to use it?

[pjebs]

The reason why it was needed is because mysql.RegisterDial("external", exSql.Dial(req, 10)) needs to be called at the start of every request cycle, hence it will get added to the dials map every time.

However, I figured out a way to put the mutexes on my side instead, so it's not needed anymore once i make the modifications to my wrapper.