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

Documentation for transaction retries is misleading #10077

Closed
aphyr opened this issue Apr 8, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@aphyr
Copy link

commented Apr 8, 2019

TiDB's documentation repeatedly claims to offer snapshot isolation. However, two automatic retry mechanisms (#10075, #10076), invalidate this claim. The first mechanism is partly documented, but the documentation is confusing. The second is undocumented. Until these mechanisms can be disabled or fixed, I suggest updating the documentation in the following ways:

  1. At the top of the transaction isolation page, claims like "TiDB implements Snapshot Isolation consistency" should include a prominent warning that this claim does not apply to current versions of TiDB; in general, transactions may exhibit lost updates due to automatic retries.

  2. "Description of optimistic transactions" currently says:

To disable the automatic retry of explicit transactions, configure the tidb_disable_txn_auto_retry global variable:

set @@global.tidb_disable_txn_auto_retry = 1;

This paragraph should explain that tidb_disable_txn_auto_retry does not disable all retries; users need to set tidb_retry_limit = 0 as well.

  1. The section "Description of optimistic transactions" sounds like it's describing the difference between optimistic and pessimistic concurrency control:

Because TiDB uses the optimistic transaction model, the final result might not be as expected if the transactions created by the explicit BEGIN statement automatically retry after meeting a conflict.

...

Under the automatic retry mechanism of TiDB, all the executed statements for the first time are re-executed again. When whether the subsequent statements are to be executed or not depends on the results of the previous statements, automatic retry cannot guarantee the final result is as expected.

Optimistic transactions do execute differently than pessimistic ones, and users commonly ask questions about the semantics of conflicts and retries in optimistic databases. In addition, many MVCC databases do provide an automatic transaction retry mechanism which does not violate snapshot isolation. These factors could lead users to misinterpret this documentation, failing to realize that the retry mechanism invalidates fundamental properties of TiDB's transactional model.

Since these are properties specifically of TiDB's retry mechanism, and not optimistic transactions in general, you might consider changing the section title from "Description of optimistic transactions" to "Transactional anomalies caused by automatic retries". I also suggest changing language like "automatic retry cannot guarantee the final result is expected" to "automatic retry can violate snapshot isolation, causing lost updates."

@morgo morgo added the type/bug label Apr 8, 2019

morgo added a commit to pingcap/docs that referenced this issue Apr 8, 2019

@morgo

This comment has been minimized.

Copy link
Member

commented Apr 8, 2019

Until these mechanisms can be disabled or fixed, I suggest updating the documentation in the following ways

Thank you for reporting this issue!

We are discussing a longer-term fix to (#10075, #10076), so as you suggest I will interpret this issue as for the documentation specifically. I have created pingcap/docs#1027 - I believe I have updated all the cases you mentioned, but appreciate a review too :-)

@morgo morgo added the component/docs label Apr 8, 2019

@morgo morgo self-assigned this Apr 8, 2019

c4pt0r added a commit to pingcap/docs that referenced this issue Apr 9, 2019

*: Improve clarification on transaction retry (#1027)
* Warn about transaction retry

From pingcap/tidb#10077

* Add warning about transaction retry

* Additional changes to clarify retry

* fix anchor link

* Add note that SI needs context

* Use TRUE consistently

Helps communicate it is a boolean even if mysql is flexible.

* Consistent spacing

* Add further clarification

* Improve case to match style
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.