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

extended ExecuteSelectStreaming #596

Merged
merged 7 commits into from
Nov 1, 2021

Conversation

skoef
Copy link
Contributor

@skoef skoef commented Jun 28, 2021

The other day I saw the memory usage of my mysql server implementation (built with go-mysql) surge to 150GB and on further inspection I learned this was caused by a SELECT-query with a rather large resultset. This spiked my interest in the recently implemented ExecuteSelectStreaming function but I had a hard time implementing anything useful with it.
My server implementation acts as a proxy between clients and multiple mysql servers. Using the ExecuteSelectStreaming function instead of Execute in for my handler's HandleQuery function lacked any way of properly streaming the resultset from the backend mysql server back to my client. This PR solves this issue for me without being too obtrusive.

Biggest change is the additional callback ExecuteSelectStreaming type SelectPerResultCallback. Once the SELECT query was successful the preliminary Result, without the actual data, is passed to this callback. This allows me to write this result directly back to the client, which at that point means only the number of fields and field information is fed back to the client followed by an EOF. Then, when rowdata comes in and therefor calls to the SelectPerRowCallback callback, I directly write these rows to the client with the new writeFieldValues function. For this to work I had to export formatTextValue and writeValue.

This is a relevant snippet of my HandleQuery where ExecuteSelectStreaming is used:

func (q MyHandler) HandleQuery(query string) (*mysql.Result, error) {
	// ...

	// I use vitess' sqlparsers for the exact same reason as #579
	if stmt == sqlparser.StmtSelect {
		// for SELECT queries, stream the result and its rows directly back to the
		// client rather than reading the whole resultset in memory and then send
		// that
		var stream mysql.Result
		err = backendConn.ExecuteSelectStreaming(query, &stream,
			// called per row within result
			func(row []mysql.FieldValue) error {
				return clientConn.WriteValue(row)
			},
			// called per result
			func(r *mysql.Result) error {
				return clientConn.WriteValue(r)
			},
		)

		return &stream, err
	} else {
		// for any query other than SELECT
		return backendConn.Execute(query)
	}
}

Using ExecuteSelectStreaming this way shows no spike or memory usage increase at all when I run the exact same query that consumed 150G of memory before.

I couldn't find any relevant examples for the use of ExecuteSelectStreaming and I know this breaks the API for ExecuteSelectStreaming but if this PR would be merged and released in a version like 1.4.0 that doesn't necessarily has to matter. I can imagine this added flexibility could make ExecuteSelectStreaming much more suitable to solve more people's problems.

Besides that, I added this functionality for prepared SELECT statements as well.

@atercattus atercattus self-assigned this Jun 30, 2021
@skoef
Copy link
Contributor Author

skoef commented Aug 16, 2021

@atercattus any chance of reviewing this? I've been running this in production for a while now without any problems so far.

@skoef
Copy link
Contributor Author

skoef commented Oct 25, 2021

@atercattus Just noticed this PR couldn't even be merged due to conflicts. I've resolved the conflict, could you review this please?

@atercattus
Copy link
Member

@skoef I'll try to review this PR today-tomorrow

@atercattus atercattus merged commit d6f5ffa into go-mysql-org:master Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants