Refresh *sql.DB object in "database/sql" package

[Abhijeet Rastogi]

Hi Gophers,

I've a complex program which translates to something like this. There are multiple goroutines that are accessing the "db" variable which is originally created in main function.

func main() {

     connString = getDBConnString()

     db := sql.Open(driver, connString)

     go func1(db)

     go func2(db)

}

The return value of getDBConnString() can change depending on what's the status of mysql cluster. 

I'm well aware that sql.DB struct has mutexes so concurrent access to this variable is allowed. But, if in any of the functions, func1 and func2, I happen to encounter any error while accessing db, I would like to issue a call to getDBConnString() and then "refresh" the db object with the new connection and close the current one. 

Is the below code right one to use?

func func1(db *sql.DB) error {

    if checkDBStatus(db) == false {

         db.Close()

         // What happens if some other goroutine access db during this time???

         db = sql.Open(driver, getDBConnString())

    }

    // Do my thing with new DB

    return nil

}

What happens if some other goroutine tries to access "db" within the timeframe when "db.Close()" and "db = sql.Open(driver, getDBConnString())" is being executed?

What's the right way to handle this kind of scenario?

Thanks in advance. 

[Kiki Sugiaman]

Before I directly answer your questions, let me first say that db should
not be closed on ordinary errors. Only close it on serious errors (the
database is unreachable anymore, etc.) or at the end of your program.
For ordinary errors, close the Rows, Stmt, Tx (via commit or rollback)
instead. With that out of the way, I'll answer inline.

The other goroutine will encounter an error. You can verify this
yourself by introducing a time delay in the other goroutine before
accessing db.

>
> What's the right way to handle this kind of scenario?

You should not be opening and closing db inside a "handler" function
(the likes of func1 and func2). No need for checkDBStatus(). The
connection object has retries built into it. You can add more retry
logic if you like, but nothing can be solved by refreshing the
connection if the underlying problem remains. The best you can do is to
log the problem.

Maybe if you could share the kind of error you expect, others could
provide more input.

(Btw, errors from func1 and func2 will not propagate up in your code,
but that's a separate issue)

>
> Thanks in advance.
>
> --
> You received this message because you are subscribed to the Google
> Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to golang-nuts...@googlegroups.com
> <mailto:golang-nuts...@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

[Jesse McNelis]

On Wed, May 18, 2016 at 4:26 AM, Abhijeet Rastogi
<abhije...@gmail.com> wrote:
> Hi Gophers,
>
> I've a complex program which translates to something like this. There are
> multiple goroutines that are accessing the "db" variable which is originally
> created in main function.
>
> func main() {
> connString = getDBConnString()
> db := sql.Open(driver, connString)
>
> go func1(db)
> go func2(db)
> }

sql.Open() doesn't just create a single connection to the database. It
creates a pool of connections.
Each time you do an sql query a connection is taken from the pool to
complete the query.
Connections that have network errors get closed and removed from the pool.

If you've got custom logic for database clustering it's probably
better to implement it as an Driver
https://golang.org/pkg/database/sql/driver/#Driver

If the clustering logic is common for a particular type of database
eg. mysql, postgresql etc. then you might consider adding it to one of
the existing driver packages for that database.

[Abhijeet Rastogi]

Hi Jessi and Kiki,

Thanks for replying. I understand that connection retries are built-in and other stuff but my use-case is different. We've in-house DBA team which maintains mysql servers. In the case of failure of master nodes, they promote other nodes as masters and this getDBConnString() is the correct way to retrieve the latest master. 

Now, I want my code to handle this use-case where the node that the db connection was initially built with has gone down and then I call this API (getDBConnString()) to get the new master and refresh the db object altogether.

To give some more context, db connection is created in main() and then these func1 and func2 are actually httpHandlers. 

Thanks,

Abhijeet

[Abhijeet Rastogi]

>Maybe if you could share the kind of error you expect, others could
provide more input.

It's complete node failure of mysql master and then the function getDBConnString() gives me a new server to work with. 

Thanks

--

Cheers,

Abhijeet Rastogi

abhijeetr.com

[Chris Kastorff]

You're right that loads and stores to that variable from different goroutines can race and cause bad issues.

It seems like an *atomic.Value (https://golang.org/pkg/sync/atomic/#Value) is appropriate for what you want to do; your loads become a little more verbose, but you can probably hide that away in a function. You'd probably also want a mutex in the updater routine so you don't create a whole bunch of new *sql.DBs and throw away all but the last.

--

You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

[Kiki Sugiaman]

In this case, a sql.DB reset is warranted. But it's better to pass the
error upstream and re-invoke the handler after repairing the connection,
since failover is supposed to be a rare edge case.

You might have to disappoint some users with an error message and have
them retry. If you can't afford that, then dealing with the race
conditions is probably unavoidable.

An alternative is to maintain multiple pools (sql.DB) and keep track of
where the master is, as well as bad/discarded and newly created pools in
real time. Then go down the list of pools until you acquire/update the
data you want. This is more resource intensive but probably presents you
with less race conditions. Remember that each additional connection is
only potentially expensive to the db but is very cheap to the go client.
You only need to maintain the # of connections per db at a manageable
level. The client can maintain any number of connections proportional to
the number of DB's you have running without a care in the world.

> abhijeetr.com <http://abhijeetr.com>

>
> --
> You received this message because you are subscribed to the Google
> Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to golang-nuts...@googlegroups.com

> <mailto:golang-nuts...@googlegroups.com>.

[Abhijeet Rastogi]

Hi Kiki,

Thanks for explaining. 

But it’s better to pass the error upstream and re-invoke the handler after repairing the connection, since failover is supposed to be a rare edge case.

Can you explain a little about what you’re suggesting I should do? “db” is created in main and then it’s passed to HTTP handlers as arguments in my case.

db, err := getDB()

and then later in code:-

    // Start web server.
    http.HandleFunc("/tokens", func(w http.ResponseWriter, r *http.Request) {
        tokensHandler(w, r, db, *envPtr)
    })
    http.HandleFunc("/add", func(w http.ResponseWriter, r *http.Request) {
        agentsHandler(w, r, db, *envPtr)

Thanks

​

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Cheers,
Abhijeet Rastogi (shadyabhi)

[Kiki Sugiaman]

By "upstream", I was making a generalization which could mean a few
things. Modifying the driver is upstream, so is handling the error in
main. I mentioned several options on how to approach your problem:
1. Display an error to your user and have the user retry.
2. Deal with the race condition a-la checkDBStatus() in your original code.
3. Something else.

In the context of (1), the handler will be re-invoked when the user
retries. Assuming you have some failover magic in place, the user will
encounter an already repaired connection the second time around.

Now....code. I'm going to use a framework to follow the pattern where
you are able to return an error from the handler function as in your
code in the OP:

https://play.golang.org/p/U4SG-wEHeV

(If you want to run the program, you need to have sqlite)

> <mailto:abhije...@gmail.com

> <mailto:abhije...@gmail.com>>> wrote:
>
> Hi Jessi and Kiki,
>
> Thanks for replying. I understand that connection retries are
> built-in and other stuff but my use-case is different. We've
> in-house DBA team which maintains mysql servers. In the case of
> failure of master nodes, they promote other nodes as
> masters and
> this getDBConnString() is the correct way to retrieve the
> latest
> master.
>
> Now, I want my code to handle this use-case where the node
> that the
> db connection was initially built with has gone down and
> then I call
> this API (getDBConnString()) to get the new master and
> refresh the
> db object altogether.
>
> To give some more context, db connection is created in
> main() and
> then these func1 and func2 are actually httpHandlers.
>
> Thanks,
> Abhijeet
>
> On Wed, May 18, 2016 at 5:16 AM Jesse McNelis
> <jes...@jessta.id.au <mailto:jes...@jessta.id.au>

> <mailto:jes...@jessta.id.au <mailto:jes...@jessta.id.au>>>

> wrote:
>
> On Wed, May 18, 2016 at 4:26 AM, Abhijeet Rastogi
> <abhije...@gmail.com

> <mailto:abhije...@gmail.com> <mailto:abhije...@gmail.com

> abhijeetr.com <http://abhijeetr.com> <http://abhijeetr.com>

>
> --
> You received this message because you are subscribed to the Google
> Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from
> it, send
> an email to golang-nuts...@googlegroups.com

> <mailto:golang-nuts%2Bunsu...@googlegroups.com>
> <mailto:golang-nuts...@googlegroups.com
> <mailto:golang-nuts%2Bunsu...@googlegroups.com>>.

> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google
> Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to golang-nuts...@googlegroups.com

> <mailto:golang-nuts%2Bunsu...@googlegroups.com>.

> For more options, visit https://groups.google.com/d/optout.
>
>
>
>
> --

> Cheers,
> Abhijeet Rastogi (shadyabhi)

[Abhijeet Rastogi]

Hi Kiki,

I still have doubts. Sorry about that.

db = validDb // some failover magic here

This operation is not atomic. Hence, while this is happening, other http handlers will also fail. I verified that by adding a time.Sleep in the example code. How can I avoid that?

Is putting mutexes around all access to db object the only way, for both reads and writes? I feel that it’s ugly and that’s why I’m looking for a better solution.

Thanks

​

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Kiki Sugiaman]

On 19/05/16 15:22, Abhijeet Rastogi wrote:
> Hi Kiki,
>

> I still have doubts. Sorry about that.
>
> |db = validDb // some failover magic here |
>
> This operation is not atomic. Hence, while this is happening, other http
> handlers will also fail.

I was trying to illustrate the handling of the failover logic outside of
the handler function (as opposed to doing checkDBStatus() each time at
the start of it). I made no attempt to illustrate a graceful failover,
hence "magic". Sorry if I wasn't clear before.

I verified that by adding a time.Sleep in the
> example code. How can I avoid that?
>
> Is putting mutexes around all access to db object the only way, for both
> reads and writes? I feel that it’s ugly and that’s why I’m looking for a
> better solution.

I'm afraid that when many goroutines are after the same resource, you
have no choice but to synchronize. Using a mutex is one way. I believe
another was suggested in this thread. Ugly is also relative, and can be
hidden by wrappers.

Feel free to consult the mailing list again if you need further help
with synchronization code.

> <mailto:golang-nuts%2Bunsu...@googlegroups.com
> <mailto:golang-nuts%252Buns...@googlegroups.com>>
> <mailto:golang-nuts...@googlegroups.com
> <mailto:golang-nuts%2Bunsu...@googlegroups.com>
> <mailto:golang-nuts%2Bunsu...@googlegroups.com
> <mailto:golang-nuts%252Buns...@googlegroups.com>>>.

> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the
> Google
> Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails
> from it,
> send an email to golang-nuts...@googlegroups.com
> <mailto:golang-nuts%2Bunsu...@googlegroups.com>

> <mailto:golang-nuts%2Bunsu...@googlegroups.com
> <mailto:golang-nuts%252Buns...@googlegroups.com>>.