Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL querying method called in two different methods makes server stops #33539

Closed
GauthierHEI opened this issue Aug 8, 2019 · 8 comments
Closed
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@GauthierHEI
Copy link

GauthierHEI commented Aug 8, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.7 linux/amd64

Does this issue reproduce with the latest release?

I believe go1.12.7 is the latest release at the moment

What operating system and processor architecture are you using (go env)?

go env
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/ghacout/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ghacout/go"
GOPROXY=""
GORACE=""
GOROOT="/snap/go/4098"
GOTMPDIR=""
GOTOOLDIR="/snap/go/4098/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/ghacout/Bureau/GraphQL_GO/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build544526689=/tmp/go-build -gno-record-gcc-switches"

What did you do?

My problem is very specific, I was able to debug it but I am creating an issue because I want to understand why it did that and I was not able to find explanation on the internet.

Context
I created a golang webserver, in this server there are Data Access Object, which fetch data from a mySQL database. In my DAO there is a method to fetch products (GetProducts) this method calls two other methods (Properties & Populate) which both call method CustomFields (which fetch database columns names specific to the customer whom products we want)

There are multiple methods involved in this process :
DAO

func GetProducts () []Product {
	
	//properties is a list of database columns names 
	properties := Properties()

        //Then using the properties I perform a SQL query on my Database and
        // it returns informations about products in form of SQL Rows in a variable "rows"

        //result is a list of products created with the informations provided by the DB
        result := Populate(rows)
 
	return result, nil
}

Methods

func Properties() []string {
	customFields := CustomFields()
        //Than we handle the customFields and returns list of database colmun names
}

func Populate(rows sql.Rows) []Product {
	customFields := CustomFields()
	//Than we handle the customFields in another way to return a list of products
}

func CustomFields() []string {
     //Get Custom fields will perform a simple SQL query on DB and return a list of custom fields name
}

Explanation
As you can see I removed a lot of code because it was just here to make you understand what's going on. There is no issue with the code and everything is working fine when I launch the server.

What did you expect to see?

A constantly working golang server

What did you see instead?

This issue is whenever is call my DAO a lot of times, about 10 requests per seconds my server is stopping. It's not dying and exiting, it's freezing and I have to relaunch it manually. This is not an infrastructure problem with the host machine or the mySQL database server.

It's due to Populate() & Properties() both calling the method CustomField(), because whenever I remove the CustomField() call in the Populate() method everything is fine. The server is working and can go up to more than 120 requests per seconds without trouble. After a lot of testing I am 100% positive that it doesn't come from anything else.

My question is : Why does calling a simple SQL query method in two different method is creating an issue ?

Thanks I hope I explained myself well and that someone will be able to help me understand what happened. If you have any questions feel free to ask

@kardianos
Copy link
Contributor

This sounds like a db locking issue. Do you close SQL.Rows? Do you defer tx.Rollback then call tx.Commit on happy path?

@GauthierHEI
Copy link
Author

Hi, Thank you for your response

I do close sql.Rows like this :

rows, err := d.sqlProvider.dbConnection.Query(query, params...)
if err != nil {
	return c, err
}
defer rows.Close()

And since there are only "SELECT" sql query (i.e I'm only reading the database) I believed tx.Rollback and tx.Commit were not useful, was I wrong ?

@kardianos
Copy link
Contributor

That sounds fine then.

Without seeing more real code and sql, it will be hard to help.

My hunch is there is a bug somewhere in your code. This doesn't sound like a database/sql issue.

@GauthierHEI
Copy link
Author

Hi and thank you again for trying to help me !

It's a bit complex because this code wasn't written by me and I don't want it to be publicly available without the writer's approval, however I can't have it right now.

But I can explain and show what it does to a certain extent.

//customFields() will return a slice of pointers (pointers to a struct named "customField")
func customFields() (CustomFields, error){

    var customFields CustomFields

    query := "SELECT ... FROM ... WHERE ... AND ... ORDER BY ... ;"	

    //sqlProvider.Get() returns a dbConnection (*sql.DB for the sql package)
    rows, err := sqlProvider.Get().Query(query)
	if err != nil {
		return c, err
	}
	defer rows.Close()

	for rows.Next() {
		var c CustomField
		if err := rows.Scan(&c.ID, &c.Name); err != nil {
			return customFields, err
		}
		customFields = append(customField, &c)
	}

	return customFields, nil
}

//Get() method returns a *sql.DB
//sqlProvider is a struct containing informations about the SQL database (user, host, port, max OpenConnections... and a map[string]*sql.DB named "Connections") 
func (s *sqlProvider)Get() {
        dbHost := s.Host
	if conn, ok := s.Connections[dbHost]; ok {
		return conn
	}

        //sql.Open will open a new connection with the sqlProvider informations
	conn, _ := sql.Open("mysql", ... )
	conn.SetMaxOpenConns(s.MaxOpenConns)
	conn.SetConnMaxLifetime(s.MaxLifetime)
	s.Connections[dbHost] = conn

	return conn
}

I hope you understood me and that you will be able to help me understand, thanks again !

@kardianos kardianos added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 12, 2019
@kardianos
Copy link
Contributor

I think I can help.

customFields looks fine in the query and and Scanning of rows.

The sqlProvider.Get looks odd, not the way I would recommend doing it. Ensure that s.Connections is guarded by a RWMutex. There isn't anything intrinsic to this code that would cause it to block. If you can repro, get into a blocked state then check how many open connections there are to MySQL and what state they are in.

@GauthierHEI
Copy link
Author

Hi, it's a very interesting point and I absolutely agree that the map[string]*sql.DB should be protected by a RWMutex in case of write/read. It will be changed ASAP !

However It cannot be the source of my problem because in my case I only use one SQL connection (meaning I create it the first time then reuse the same one all the time). I checked with metrics on mySQL server and there is only one open SQL connection as expected.

Thanks again for your help !

@lucasmoten
Copy link

The symptoms sound like out of available connections and the engine blocking waiting for one to free up.

While details are missing, it looks to me like your code is opening a db connection in GetProducts and passes the rows to populate. This connection is still open when subsequently calling customFields which is in need of an available database connection to proceed. Until your single connection closes it'll block, but since its single threaded, it has no hope of that. Even if killed from the server, the client pool of one connection will think it's still in use.

You may need to use a connection pool of multiple connections instead of a single one from the map tied to dbhost.
In addition, close the connection, or release to the pool, as soon as possible.

@mvdan
Copy link
Member

mvdan commented Jun 15, 2021

Closing old issues that still have the WaitingForInfo label where enough details to investigate weren't provided. Feel free to leave a comment with more details and we can reopen.

@mvdan mvdan closed this as completed Jun 15, 2021
@golang golang locked and limited conversation to collaborators Jun 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants