Skip to content

Shrink NET::buff to reduce memory usage - #714

Open
SongLibing wants to merge 1 commit into
mysql:trunkfrom
SongLibing:net_buffer_shrink
Open

Shrink NET::buff to reduce memory usage#714
SongLibing wants to merge 1 commit into
mysql:trunkfrom
SongLibing:net_buffer_shrink

Conversation

@SongLibing

Copy link
Copy Markdown

Problem:

NET::buff grows to accommodate large incoming packets but does not return to its initial size. Long-lived connections can therefore retain excess memory after processing occasional large requests.

Solution:

Add the net_buffer_shrink_threshold session variable. When the current packet uses no more than half of an expanded buffer, count a shrink request. After the configured number of consecutive requests, halve the buffer without shrinking below net_buffer_length or IO_SIZE. Preserve the existing network error state if the allocation fails.

Copyright (c) 2026, Oracle and/or its affiliates.

What does this change do?

Optimize the memory usage of net::buff
mysql/mysql-community#78

Why is it needed?

It is needed to reduce memory usage and remain the server stable.

How was it tested?

  • Added/updated MTR tests under mysql-test/
  • scripts/ci/mtr.sh passes locally
  • Ran the relevant full suite (name it): ______

Contributor checklist

  • I have signed the OCA with the email on these commits
  • Code is formatted (scripts/ci/format.sh)
  • Commits are focused with descriptive messages

AI assistance

  • I did not use AI assistance for this contribution
  • I used AI assistance for this contribution

If AI assistance was used, describe the tool(s) and extent of use:
codex, merge the code to mysql-trunk

Areas touched

NET, THD

@SongLibing
SongLibing requested a review from a team August 6, 2026 21:10
@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 6, 2026
@github-actions github-actions Bot added tests Changes touching test code or test data MTR Failed MTR suite failed Build Failed PR build failed labels Aug 6, 2026
@gopshank
gopshank requested review from harinvadodaria and phulakun and removed request for gopshank and seemasundara August 7, 2026 15:50
@github-actions github-actions Bot added the Review Requested Review requested from code owners label Aug 10, 2026
FROM performance_schema.threads
WHERE PROCESSLIST_ID = @con1_processlist_id);

# =============================================================================

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Kindly add test to verify behavior with small queries less than net_buffer_shrink_threshold.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For test2 and test3, I added query to show the allocated buffer, just after the forth SELECT 1. the allocated buffer should no change.

Comment thread sql/sql_class.cc Outdated
uchar old_error = net->error;

if (net_realloc(net, target_buffer_capacity)) {
net->last_errno = old_last_errno;

@phulakun phulakun Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If net_realloc() hits OOM:
my_realloc(..., MYF(MY_WME)) reports EE_OUTOFMEMORY through my_error().
This can populate the current THD diagnostics area.

Restoring net->error and net->last_errno repairs only the NET state. The THD diagnostics area can still hold OOM.

Perhaps shrink should call my_realloc(..., MYF(0)) instead. Something like,

// sql-common/net_serv.cc
// Note: net_shrink is a new function.
bool net_shrink(NET *net, size_t length) {
assert(net->write_pos == net->buff);
assert(length < net->max_packet);

const size_t target =
(length + IO_SIZE - 1) & ~(static_cast<size_t>(IO_SIZE) - 1);

uchar *buff = static_cast<uchar *>(my_realloc(
key_memory_NET_buff, net->buff,
target + NET_HEADER_SIZE + COMP_HEADER_SIZE, MYF(0)));

if (buff == nullptr) return true;

net->buff = buff;
net->buff_end = buff + target;
net->write_pos = buff;
net->read_pos = buff;
net->max_packet = static_cast(target);
return false;
}

What do you think?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added net_shrink function into net_srv.cc, net_shrink doesn't report any error.

Comment thread sql/sys_vars.cc Outdated
"If required size of NET::buff currently is less than half the allocated "
"one, shrink will be requested. When number of consecutive requests is "
"equal or greater than this value, shrink will be triggered. 0 means "
"never do shrink.",

@phulakun phulakun Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps its good to mention that NET::buff is not shrinked below net_buffer_length.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure, added.

@phulakun phulakun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello SongLibing!

Thank you for working on this. It looks good. Please find few comments.

Regards,
Praveen

@SongLibing

SongLibing commented Aug 12, 2026

Copy link
Copy Markdown
Author

Hello SongLibing!

Thank you for working on this. It looks good. Please find few comments.

Regards, Praveen

Hi Praveen,

Thanks for your review comments. I addressed all problems you mentioned. Please have a look.

Regards, Libing

@phulakun phulakun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello Libing!

Thank you for addressing review comments.
Changes looks good to me. Kindly wait for review from Harin.

Regards,
Praveen

@harinvadodaria harinvadodaria left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi Libing,

Thanks for submitting the patch.
Please see some comments below

Comment thread include/mysql_com.h Outdated
*/
void *extension;
/** Number of consecutive requests to shrink NET::buff. */
unsigned int shrink_requests;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Adding a new variable in NET would change sizeof(NET) and in turn sizeof(MYSQL) because NET is part of MYSQL structure. This is also captured by changes in mysql.h.pp. Such a change would need a bump in libmysql version.

Instead, please extend structure NET_SERVER in mysql_com_server.h and initialise in init_net_server_extension().

This way, ABI changes are avoided and no version bump is needed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Got it, thanks! moved it into NET_SERVER.

Comment thread sql/sys_vars.cc
VALID_RANGE(1024, 1024 * 1024), DEFAULT(16384), BLOCK_SIZE(1024),
NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_net_buffer_length));

#define DEFAULT_NET_BUFFER_SHRINK_THRESHOLD 5

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the rationale behind this default value? While I understand that shrinking a buffer reduces memory usage. It also means that every now and then, perf penalty is paid to realloc required buffer. Can you share some performance data based on your observations?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The target is to reduce memory usage for very large net buffer. The feature is enabled in production by default, we use a relative mild config value 5 with cautious. Because we don't want the config impact the instances in which queries are often large than net_buffer_length. And the value 5 works fine in production. Below is an real case we observed in production : you can see the memory usage is much less and the instance stays stable after the server is updated to the version with this feature.
image

Comment thread include/mysql_com.h Outdated
void net_clear(struct NET *net, bool check_buffer);
void net_claim_memory_ownership(struct NET *net, bool claim);
bool net_realloc(struct NET *net, size_t length);
bool net_shrink(struct NET *net, size_t length);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once shrink_request is moved out of NET, this function can be removed from mysql_com.h.
Instead, it can be part of server code itself.

@SongLibing SongLibing Aug 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Moved it into mysql_com_server.h

Comment thread sql/sql_parse.cc Outdated
if (net->max_packet > thd->variables.net_buffer_length &&
net->max_packet > IO_SIZE &&
thd->variables.net_buffer_shrink_threshold != 0) {
thd->try_shrink_net_buffer(net, protocol->get_packet_length());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All of the above mentioned checks should be folded into try_shrink_net_buffer(). You already have assertions for same conditions in the function.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is originally in the try function. I moved the check out try_shrink_net_buffer, because I think it will avoid a function call in most of the situation. So it is faster.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Folded into try_shrink_net_buffer(). The function is only called once in the query life, it is fine.

Comment thread sql-common/net_serv.cc Outdated
net->last_errno = 0;
#ifdef MYSQL_SERVER
net->extension = nullptr;
net->shrink_requests = 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be removed once shrink_requests is moved as suggested in one of the previous comments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

removed

Comment thread sql-common/net_serv.cc
@retval true failed to shrink
@retval false Shrunk to length successfully
*/
bool net_shrink(NET *net, size_t length) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please fold it into try_shrink_net_buffer() or define a separate function within server layer itself.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added #ifdef MYSQL_SERVER to make it server only.

@SongLibing

Copy link
Copy Markdown
Author

Hi @harinvadodaria ,
Thanks for the great suggestions. I addressed all problems. Please have a look.

Problem
=======
NET::buff grows to accommodate large incoming packets but does not return
to its initial size. Long-lived connections can therefore retain excess
memory after processing occasional large requests.

Solution
========
Add the net_buffer_shrink_threshold session variable. When the current
packet uses no more than half of an expanded buffer, count a shrink
request. After the configured number of consecutive requests, halve the
buffer without shrinking below net_buffer_length or IO_SIZE. Preserve the
existing network error state if the allocation fails.

@harinvadodaria harinvadodaria left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the updated patch, @SongLibing .
Approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Failed PR build failed MTR Failed MTR suite failed OCA Verified All contributors have signed the Oracle Contributor Agreement. Review Requested Review requested from code owners tests Changes touching test code or test data

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants