sql: resolve databases in high-priority transactions #46384
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
This commit is a follow up to cockroachdb#46170. That PR dealt with the table resolution self-deadlock and additionally ensures that lease acquisition is not blocked. That commit did not deal with the fact that schema changes which mutate databases can deadlock themselves on retry because a separate transaction is used to resolve database descriptors during planning. This patch makes reading the system.namespace table and reading database descriptors in high-priority transactions. The transactions will push locks owned by schema changes out of their way. The idea is that regular SQL transactions reading schema don't want to wait for the transactions performing DDL statements. Instead, those DDLs will be pushed and forced to refresh. Besides the benefit to regular transactions, this patch also prevents deadlocks for the transactions performing DDL. Before this patch, the final select in the following sequence would deadlock: begin; savepoint s; create database d; rollback to savepoint s; select * from d.t; The select is reading the namespace table, using a different txn from its own. That read would block on the intent laid down by the prior create. With this patch, the transaction effectively pushes itself, but gets to otherwise run. Fixes cockroachdb#46224 Release justification: Fix for an old bug that became more preminent when we introduced SAVEPOINTs recently. Release note (bug fix): A rare bug causing transactions that have performed schema changes to deadlock after they restart has been fixed.
ajwerner
force-pushed
the
ajwerner/fix-deadlock-in-database-resolution
branch
from
4eab36a to
7caf797
Compare
|
LGTM |
|
TFTR! bors r=andreimatei |
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit is a follow up to #46170. That PR dealt with the table resolution
self-deadlock and additionally ensures that lease acquisition is not blocked.
That commit did not deal with the fact that schema changes which mutate
databases can deadlock themselves on retry because a separate transaction
is used to resolve database descriptors during planning.
This patch makes reading the system.namespace table and reading database
descriptors in high-priority transactions. The transactions will push
locks owned by schema changes out of their way. The idea is that regular
SQL transactions reading schema don't want to wait for the transactions
performing DDL statements. Instead, those DDLs will be pushed and forced
to refresh.
Besides the benefit to regular transactions, this patch also prevents
deadlocks for the transactions performing DDL. Before this patch, the
final select in the following sequence would deadlock:
begin; savepoint s; create database d; rollback to savepoint s;
select * from d.t;
The select is reading the namespace table, using a different txn from
its own. That read would block on the intent laid down by the prior
create. With this patch, the transaction effectively pushes itself, but
gets to otherwise run.
Fixes #46224
Release justification: Fix for an old bug that became more preminent
when we introduced SAVEPOINTs recently.
Release note (bug fix): A rare bug causing transactions that have performed
schema changes to deadlock after they restart has been fixed.