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

Support configuring session variable in lightning logical backend #47305

Closed
guoshouyan opened this issue Sep 26, 2023 · 5 comments · Fixed by #47414
Closed

Support configuring session variable in lightning logical backend #47305

guoshouyan opened this issue Sep 26, 2023 · 5 comments · Fixed by #47414
Assignees
Labels
type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@guoshouyan
Copy link
Contributor

Feature Request

Can lightning support specifying session variables to use in lightning config?

In our case, we want to disable the async_commit in lightning, that is because during our test on stale read on TiDB cluster, we find out that async_commit could block stale_ts, which will block the whole tikv node. Since this issue will not be solved in a short time, we want to disable the async_commit in lightning for now and here is my pr: #46961

But I think we can generalize it by enabling lightning to let users specify a lit of session variable in the config that they want to use.

What is your thinking on this?

Is your feature request related to a problem? Please describe:

Describe the feature you'd like:

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

@guoshouyan guoshouyan added the type/feature-request Categorizes issue or PR as related to a new feature. label Sep 26, 2023
@lance6716
Copy link
Contributor

Hi, I found in the last we already proposed to set lightning session variables #28471 , but only limited to the scenario that lightning is used as a library. Maybe now we can change

	Vars                       map[string]string `toml:"-" json:"vars"`

to

	Vars                       map[string]string `toml:"session-vars" json:"vars"`

? Do you think it's acceptable?

@guoshouyan
Copy link
Contributor Author

Hi, I found in the last we already proposed to set lightning session variables #28471 , but only limited to the scenario that lightning is used as a library. Maybe now we can change

	Vars                       map[string]string `toml:"-" json:"vars"`

to

	Vars                       map[string]string `toml:"session-vars" json:"vars"`

? Do you think it's acceptable?

Yeah, I think it should work in our case.

@guoshouyan
Copy link
Contributor Author

Hi @lance6716 are you making the changes or you need me to make the changes?

@lance6716
Copy link
Contributor

lance6716 commented Oct 4, 2023

@guoshouyan I'm on holiday this week and haven't start to make the change. I also don't have a strict plan to assign this issue to you or me, we can use our spare time and check the linked PR of this issue to know if you or me have started, and I guess I will not come to work before Oct 08

@lance6716
Copy link
Contributor

/assign @lance6716

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants