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

Make token-limit configurable as an INSTANCE scoped variable #34583

Open
morgo opened this issue May 11, 2022 · 5 comments
Open

Make token-limit configurable as an INSTANCE scoped variable #34583

morgo opened this issue May 11, 2022 · 5 comments
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@morgo
Copy link
Contributor

morgo commented May 11, 2022

Enhancement

This is forked from #33769

For token-limit there is a use-case to make it online configurable, but it's not a great fit for GLOBAL. This enhancement proposes that we add an instance scoped variable for it.

For the name, I suggest tidb_max_connection_concurrency or tidb_connection_concurrency_limit.

This is different from the issues in #33769 because it is not a breaking change. It can still be configured in the config file as token-limit or via the instance scoped variable that we are introducing.

@morgo morgo added the type/enhancement The issue or PR belongs to an enhancement. label May 11, 2022
@morgo
Copy link
Contributor Author

morgo commented May 11, 2022

@CbcWestwolf would you like to take a look at this?

@CbcWestwolf
Copy link
Member

@CbcWestwolf would you like to take a look at this?

Sure! Let me do this convertion.

@CbcWestwolf
Copy link
Member

/assign

@CbcWestwolf
Copy link
Member

It can still be configured in the config file as token-limit or via the instance scoped variable that we are introducing.

Should we move the token-limit to [instance] section in config file just like what we have done in #33279?

@morgo
Copy link
Contributor Author

morgo commented May 14, 2022

It can still be configured in the config file as token-limit or via the instance scoped variable that we are introducing.

Should we move the token-limit to [instance] section in config file just like what we have done in #33279?

Yes. Under [instance] it should have the same name as the sysvar. For the sysvar name, I suggested tidb_max_connection_concurrency or tidb_connection_concurrency_limit. The problem with "token limit" is it is too tied to an implementation, not what the feature does (a cap on concurrent connections that can be executing).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
2 participants