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

tidb_txn_mode don't default to pessimistic #20990

Closed
erikthorselius opened this issue Nov 11, 2020 · 9 comments
Closed

tidb_txn_mode don't default to pessimistic #20990

erikthorselius opened this issue Nov 11, 2020 · 9 comments
Labels
severity/moderate sig/transaction SIG:Transaction type/question The issue belongs to a question.

Comments

@erikthorselius
Copy link

erikthorselius commented Nov 11, 2020

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

docker run -d -p 4000:4000 pingcap/tidb:v4.0.8
mysql -h localhost -P 4000 -u root --protocol tcp -e "show global variables where Variable_name='tidb_txn_mode'

2. What did you expect to see? (Required)

+---------------+-------------+
| Variable_name | Value       |
+---------------+-------------+
| tidb_txn_mode | pessimistic |
+---------------+-------------+

3. What did you see instead (Required)

+---------------+-------+
| Variable_name | Value |
+---------------+-------+
| tidb_txn_mode |       |
+---------------+-------+

4. What is your TiDB version? (Required)

| Release Version: v4.0.8
Edition: Community
Git Commit Hash: 66ac9fc31f1733e5eb8d11891ec1b38f9c422817
Git Branch: heads/refs/tags/v4.0.8
UTC Build Time: 2020-10-30 08:21:16
GoVersion: go1.13
Race Enabled: false
TiKV Min Version: v3.0.0-60965b006877ca7234adaced7890d7b029ed1306
Check Table Before Drop: false |
@erikthorselius erikthorselius added the type/bug The issue is confirmed as a bug. label Nov 11, 2020
@zyguan
Copy link
Contributor

zyguan commented Nov 11, 2020

Thanks for the report, @erikthorselius , tidb_txn_mode defaults to pessimistic when the store is tikv, however, docker run -d -p 4000:4000 pingcap/tidb:v4.0.8 starts a tidb with mocktikv.

@erikthorselius
Copy link
Author

It would be nice to have a mock that have similar configuration as to the real store or maybe make it configurable from start. We have a library that checks that the database have the right configuration before it can be uses. It works well when accessing a bigger installation (read dev/prod environments) of tidb but not when just running on a developers machine.

@zyguan
Copy link
Contributor

zyguan commented Nov 12, 2020

Yes, you are right, we keep it unchanged when using mocktikv because many test code rely on it. Can set global tidb_txn_mode='pessimistic'; during provisioning resolve the problem?

@erikthorselius
Copy link
Author

That's is how I solved it in the test's around the library right now. But it is a bit annoying because it takes around 2 seconds for the change to apply. It would be nice if it could be configured via configuration.

@zyguan
Copy link
Contributor

zyguan commented Nov 13, 2020

@cfzjywxk @nullnotnil How do you think about this? Do we need to add a configuration item for it, or are there any other solutions?

@ghost
Copy link

ghost commented Nov 16, 2020

@cfzjywxk @nullnotnil How do you think about this? Do we need to add a configuration item for it, or are there any other solutions?

I was not aware that the tests rely on optimistic, but that makes sense based on history. Without knowing how much work is involved to switch the tests, I agree with @erikthorselius that mocktikv should be using the same default. Otherwise there might be bugs in the default configuration which are not caught?

@erikthorselius
Copy link
Author

Is it possible to take any decision? I'm just a humble user of your great software, I have a clunky workaround that I can live with. But maybe this could help other users.

@cfzjywxk
Copy link
Contributor

cfzjywxk commented Nov 26, 2020

Is it possible to take any decision? I'm just a humble user of your great software, I have a clunky workaround that I can live with. But maybe this could help other users.

@erikthorselius
I think it should be changed to the same, the problem is that unit test cases may fail becasuse they depend on the old default value and we may need to refactor these cases, as @nullnotnil said. It may need some time to check them all.
The set statment set global tidb_txn_mode = 'pessimsitic' could be used to change it currently before other statement executions.

@zyguan zyguan added type/question The issue belongs to a question. and removed type/bug The issue is confirmed as a bug. labels Dec 23, 2021
@serbaut
Copy link

serbaut commented Oct 3, 2022

Fixed in #36578

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/moderate sig/transaction SIG:Transaction type/question The issue belongs to a question.
Projects
None yet
Development

No branches or pull requests

6 participants