Skip to content

improve query logging efficiency by passing io.Writer along#4122

Merged
sougou merged 2 commits intovitessio:masterfrom
tinyspeck:improve-query-logging-efficiency
Aug 12, 2018
Merged

improve query logging efficiency by passing io.Writer along#4122
sougou merged 2 commits intovitessio:masterfrom
tinyspeck:improve-query-logging-efficiency

Conversation

@demmer
Copy link
Copy Markdown
Member

@demmer demmer commented Aug 6, 2018

Description

Change the internal interface for query logging so that it passes an io.Writer into the formatter function as opposed to producing a temporary string and calling io.WriteBytes.

This should reduce some GC pressure and enable more efficient buffer management under heavy workloads.

Along the way extract the common functionality for formatting bind variables out from both vtgate and vttablet's query logging modules into a sqltypes.FormatBindVariables helper.

Testing

On a couple of quick before-and-after tests in our dev environment I couldn't discern any meaningful CPU performance impact, but I wasn't sure how to measure the GC impact. In principle I can't think of any reason why this would be worse, though it might not actually be meaningfully better.

If this looks reasonable then we (Slack) can try it out in a more systematic way to see if it has any measurable effect.

demmer added 2 commits August 6, 2018 10:09
Extract the common functionality for formatting bind variables
out from both vtgate and vttablet's query logging modules into
a sqltypes.FormatBindVariables helper.

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>
Change the internal interface for query logging so that it passes an
io.Writer into the formatter function as opposed to producing a
temporary string and calling io.WriteBytes.

This should reduce some GC pressure and enable more efficient buffer
management under heavy workloads.

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>
@demmer demmer requested a review from sougou August 6, 2018 17:13
@derekperkins
Copy link
Copy Markdown
Member

I'm not sure what your appetite / ability is for external platforms, but we monitor our GC with https://cloud.google.com/profiler/ and it works great.

Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good cleanup. So, it doesn't need a noticeable performance impact.

@sougou sougou merged commit 1ff8c32 into vitessio:master Aug 12, 2018
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.

3 participants