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

No throttle when throttle_max_bytes_per_second_to_client == 0 #1953

Merged
merged 1 commit into from Mar 16, 2019

Conversation

Projects
None yet
3 participants
@carsonip
Copy link
Contributor

commented Mar 14, 2019

If throttle_max_bytes_per_second_to_client == 0, disable throttling.
This matches the mysql convention of 0 == no limit.

@pondix

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2019

Automated message: PR pending admin approval for build testing

@carsonip

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

@renecannao

  1. Should we change the default to 0? Although I think the current default is OK.
  2. Should we change total_written in https://github.com/sysown/proxysql/pull/1953/files#diff-6535427570d12b480d5c0d5f83c0641eR468 to unsigned long long? It looks possible that this int may overflow. But it will not affect anything if throttle_max_bytes_per_second_to_client is 0.
@renecannao

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2019

@carsonip:

  1. I like the idea of changing default to 0
  2. I don't think is needed. Because of this , total_written shouldn't be able to exceed INT_MAX/10 + a network buffer. Thoughts?
@carsonip

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

2. I don't think is needed. Because of [this](https://github.com/sysown/proxysql/pull/1953/files#diff-6535427570d12b480d5c0d5f83c0641eR496) , `total_written` shouldn't be able to exceed INT_MAX/10 + a network buffer. Thoughts?

Right. It will only overflow when throttling is disabled, but it doesn't matter.

The next thing to do is to update the wiki: https://github.com/sysown/proxysql/wiki/Global-variables#mysql-throttle_max_bytes_per_second_to_client

Also I have changed the default of the variable. Please merge this PR if it looks good to you.

@carsonip carsonip force-pushed the carsonip:fix-throttle branch from c99e4da to c8995da Mar 15, 2019

No throttle when throttle_max_bytes_per_second_to_client == 0
If throttle_max_bytes_per_second_to_client == 0, disable throttling.
This matches the mysql convention of 0 == no limit. Change the default
to 0.

@carsonip carsonip force-pushed the carsonip:fix-throttle branch from c8995da to cfe1c7f Mar 15, 2019

@renecannao renecannao merged commit 48e49a1 into sysown:v2.0.3 Mar 16, 2019

@renecannao

This comment has been minimized.

Copy link
Contributor

commented Mar 16, 2019

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.