Merge #46384

46384: sql: resolve databases in high-priority transactions r=andreimatei a=ajwerner

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.

Co-authored-by: Andrew Werner <[email protected]>

pkg/sql/database.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/config"
 	"github.com/cockroachdb/cockroach/pkg/kv"
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -178,6 +179,14 @@ func (dc *databaseCache) getDatabaseDesc(
 	}
 	if desc == nil {
 		if err := txnRunner(ctx, func(ctx context.Context, txn *kv.Txn) error {
+			// Run the descriptor read as high-priority, thereby pushing any intents out
+			// of its way. We don't want schema changes to prevent database lookup;
+			// we'd rather force them to refresh. Also this prevents deadlocks in cases
+			// where the name resolution is triggered by the transaction doing the
+			// schema change itself.
+			if err := txn.SetUserPriority(roachpb.MaxUserPriority); err != nil {
+				return err
+			}
 			a := UncachedPhysicalAccessor{}
 			desc, err = a.GetDatabaseDesc(ctx, txn, name,
 				tree.DatabaseLookupFlags{Required: required})
@@ -222,6 +231,14 @@ func (dc *databaseCache) getDatabaseID(
 	}
 	if dbID == sqlbase.InvalidID {
 		if err := txnRunner(ctx, func(ctx context.Context, txn *kv.Txn) error {
+			// Run the namespace read as high-priority, thereby pushing any intents out
+			// of its way. We don't want schema changes to prevent database acquisitions;
+			// we'd rather force them to refresh. Also this prevents deadlocks in cases
+			// where the name resolution is triggered by the transaction doing the
+			// schema change itself.
+			if err := txn.SetUserPriority(roachpb.MaxUserPriority); err != nil {
+				return err
+			}
 			var err error
 			dbID, err = getDatabaseID(ctx, txn, name, required)
 			return err

pkg/sql/logictest/testdata/logic_test/schema_change_in_txn

@@ -1777,7 +1777,7 @@ COMMIT
 # namespace entries could block the schema resolution after the rollback (schema
 # resolution uses different transactions to do its reads). We've fixed it by having those
 # other transactions run at high priority, thus pushing the intents out of their way.
-subtest no_schemachange_deadlock_after_savepoint_rollback
+subtest no_table_schemachange_deadlock_after_savepoint_rollback
 
 statement ok
 begin; savepoint s; create table t(x int); rollback to savepoint s;
@@ -1787,3 +1787,14 @@ select * from t;
 
 statement ok
 commit;
+
+subtest no_database_schemachange_deadlock_after_savepoint_rollback
+
+statement ok
+begin; savepoint s; create database d46224; rollback to savepoint s;
+
+query error  relation "d46224.t" does not exist
+select * from d46224.t;
+
+statement ok
+commit;