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

release-2.1: storage: don't clear lastTxnMeta on WriteIntentError to different key #32853

Merged

Conversation

nvanbenschoten
Copy link
Member

Backport 1/1 commits from #32773.

/cc @cockroachdb/release


Fixes #32582.

This change removes a faulty optimization in the contentionQueue.
The optimization removed the txnMeta associated with a contended key
in the queue when it found a WriteIntentError from a different key.
It didn't take into account that this error could be from an earlier
request within the same batch, meaning that we can't make any assumptions
about the state of the previously contended intent simply because we
see a different WriteIntentError.

Release note (bug fix): Fix a bug where metadata about contended keys
was inadvertently ignored, allowing for a failure in txn cycle detection
and transaction deadlocks in rare cases.

Fixes cockroachdb#32582.

This change removes a faulty optimization in the `contentionQueue`.
The optimization removed the txnMeta associated with a contended key
in the queue when it found a `WriteIntentError` from a different key.
It didn't take into account that this error could be from an earlier
request within the same batch, meaning that we can't make any assumptions
about the state of the previously contended intent simply because we
see a different `WriteIntentError`.

Release note (bug fix): Fix a bug where metadata about contended keys
was inadvertently ignored, allowing for a failure in txn cycle detection
and transaction deadlocks in rare cases.
@nvanbenschoten nvanbenschoten requested review from bdarnell and a team December 5, 2018 17:12
@cockroach-teamcity
Copy link
Member

This change is Reviewable

nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this pull request Dec 5, 2018
Saw in CI of cockroachdb#32853.

We were ignoring deadlock errors on txn4, but not txn5.

Release note: None
@nvanbenschoten
Copy link
Member Author

Waiting on #32864.

craig bot pushed a commit that referenced this pull request Dec 5, 2018
32864: storage: fix flake in TestContendedIntentChangesOnRetry r=nvanbenschoten a=nvanbenschoten

Saw in CI of #32853.

We were ignoring deadlock errors on txn4, but not txn5.

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Saw in CI of cockroachdb#32853.

We were ignoring deadlock errors on txn4, but not txn5.

Release note: None
@nvanbenschoten
Copy link
Member Author

Cherry-picked #32864. Merging on green.

@nvanbenschoten nvanbenschoten merged commit 8d807ce into cockroachdb:release-2.1 Dec 5, 2018
@nvanbenschoten nvanbenschoten deleted the backport2.1-32773 branch December 5, 2018 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants