DB connection not returned to connection pool if failed to acquire the lock

[vanhuycntt]

I got my application hung after doing a load test. I dug into the library code and see that the database connection is not closed (return connection to the connection pool) if unable to obtain the lock.

[sanketplus]

I think you are right. Connection obtained here https://github.com/sanketplus/go-mysql-lock/blob/master/locker_client.go#L59 is not closed if there is any issue with query execution or return code is unexpected. More than happy to review a PR if you are interested :)

[sanketplus]

Slightly unrelated but we are not using connection pool here, rather we fetch/create an explicit connection and close it manually https://pkg.go.dev/database/sql#Conn.

Since a lock is held by a db connection, we do not want the connection to get back to the pool and still (unintentionally) hold the lock.

[vanhuycntt]

Thank you for your explanation. I think we can fix it by returning the connection to the pool when got any issue with query execution or return code is unexpected like :

 dbConn, err := l.db.Conn(ctx)
 row := dbConn.QueryRowContext(ctx, "SELECT COALESCE(GET_LOCK(?, ?), 2)", key, timeout)

 var res int
 err = row.Scan(&res)
 if err != nil || res != 1 {
    defer func() {
        dbConn.Close()
    }
    // handle error or unexpected code return from MySQL as normal
    ....
 }
...	
	

Is it fine?

[ankur-a]

@sanketplus We actually get the connection from the connection pool, https://pkg.go.dev/database/sql#DB.Conn, we can add defer dbConn.Close() soon after successfully getting a connection should help to manage all the places from where we return execution.

[sanketplus]

apologies for the delayed response, I got busy with certain life events :)

@vanhuycntt doing just defer dbConn.Close() in case of error should be okay.

@ankur-a since a lock is tied to a connection, we cannot release the connection until lock is no longer needed.

[jiahui-grabtaxi]

@sanketplus hello, we are using this awesome library in our application but we are facing this issue as we use this library in one of our long running tasks. created a PR here.. do you mind to spend some time to review and give feedback? thanks in advance. 🙏

[sanketplus]

closed in #13

[jiahui-grabtaxi]

@sanketplus sorry to disturb you again... do you mind to do a release say 0.0.7 for this fixes? it would help on importing to our go module 😄

[sanketplus]

@jiahui911 done now. apologies I missed that :)