@@ -12,18 +12,15 @@ import (
1212 "time"
1313
1414 "github.com/cockroachdb/cockroach/pkg/base"
15- "github.com/cockroachdb/cockroach/pkg/testutils"
1615 "github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
1716 "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
1817 "github.com/cockroachdb/cockroach/pkg/util/leaktest"
1918 "github.com/cockroachdb/cockroach/pkg/util/log"
2019 "github.com/stretchr/testify/require"
2120)
2221
23- // TestAllowRoleMembershipsToChangeDuringTransaction ensures that when the
24- // corresponding session variable is set for all sessions using cached
25- // role memberships, performing new grants do not need to wait for open
26- // transactions to complete.
22+ // TestAllowRoleMembershipsToChangeDuringTransaction ensures user operations
23+ // are not blocked by transactions with the corresponding session variable set.
2724func TestAllowRoleMembershipsToChangeDuringTransaction (t * testing.T ) {
2825 defer leaktest .AfterTest (t )()
2926 defer log .Scope (t ).Close (t )
@@ -40,14 +37,15 @@ func TestAllowRoleMembershipsToChangeDuringTransaction(t *testing.T) {
4037 }
4138 }
4239
43- // Create three users: foo, bar, and baz.
40+ // Create four users: foo, bar, biz , and baz.
4441 // Use one of these users to hold open a transaction which uses a lease
4542 // on the role_memberships table. Ensure that initially granting does
4643 // wait on that transaction. Then set the session variable and ensure
4744 // that it does not.
4845 tdb := sqlutils .MakeSQLRunner (sqlDB )
4946 tdb .Exec (t , "CREATE USER foo PASSWORD 'foo'" )
5047 tdb .Exec (t , "CREATE USER bar" )
48+ tdb .Exec (t , "CREATE USER biz" )
5149 tdb .Exec (t , "CREATE USER baz" )
5250 tdb .Exec (t , "GRANT admin TO foo" )
5351 tdb .Exec (t , "CREATE DATABASE db2" )
@@ -56,20 +54,27 @@ func TestAllowRoleMembershipsToChangeDuringTransaction(t *testing.T) {
5654 fooDB , cleanupFoo := openUser ("foo" , "db2" )
5755 defer cleanupFoo ()
5856
59- // In this first test, we want to make sure that the query cannot proceed
60- // until the outer transaction commits. We launch the grant while the
61- // transaction is open, wait a tad, make sure it hasn't made progress,
62- // then we commit the relevant transaction and ensure that it does finish.
57+ // Ensure that the outer transaction blocks the user admin operations until
58+ // it is committed and its lease on the initial versions of the tables are
59+ // released.
6360 t .Run ("normal path waits" , func (t * testing.T ) {
64- fooTx , err := fooDB .BeginTx (ctx , nil )
61+ outerTx , err := fooDB .BeginTx (ctx , nil )
6562 require .NoError (t , err )
63+ defer func () { _ = outerTx .Rollback () }()
64+
6665 var count int
6766 require .NoError (t ,
68- fooTx .QueryRow ("SELECT count(*) FROM t" ).Scan (& count ))
67+ outerTx .QueryRow ("SELECT count(*) FROM t" ).Scan (& count ))
6968 require .Equal (t , 0 , count )
69+
70+ // the first user operation shouldn't block
71+ _ , err = sqlDB .Exec ("GRANT biz TO bar;" )
72+ require .NoError (t , err )
73+
74+ // the outer tx still has a lease on the initial table version so this should block
7075 errCh := make (chan error , 1 )
7176 go func () {
72- _ , err : =sqlDB .Exec ("GRANT bar TO baz ;" )
77+ _ , err = sqlDB .Exec ("GRANT baz TO bar ;" )
7378 errCh <- err
7479 }()
7580 select {
@@ -79,74 +84,38 @@ func TestAllowRoleMembershipsToChangeDuringTransaction(t *testing.T) {
7984 case err := <- errCh :
8085 t .Fatalf ("expected transaction to block, got err: %v" , err )
8186 }
82- require .NoError (t , fooTx .Commit ())
87+ require .NoError (t , outerTx .Commit ())
8388 require .NoError (t , <- errCh )
8489 })
8590
86- // In this test we ensure that we can perform role grant and revoke
87- // operations while the transaction which uses the relevant roles
88- // remains open. We ensure that the transaction still succeeds and
89- // that the operations occur in a timely manner.
90- t .Run ("session variable prevents waiting during GRANT and REVOKE" , func (t * testing.T ) {
91+ // When the session variable is set, the outer transaction should not block
92+ // the user admin operations as it releases the leases after every
93+ // statement.
94+ t .Run ("session variable prevents blocking" , func (t * testing.T ) {
9195 fooConn , err := fooDB .Conn (ctx )
9296 require .NoError (t , err )
9397 defer func () { _ = fooConn .Close () }()
94- _ , err = fooConn .ExecContext (ctx , "SET allow_role_memberships_to_change_during_transaction = true;" )
98+
99+ outerTx , err := fooConn .BeginTx (ctx , nil )
95100 require .NoError (t , err )
96- fooTx , err := fooConn .BeginTx (ctx , nil )
101+ defer func () { _ = outerTx .Rollback () }()
102+
103+ _ , err = outerTx .ExecContext (ctx , "SET allow_role_memberships_to_change_during_transaction = true;" )
97104 require .NoError (t , err )
105+
98106 var count int
99107 require .NoError (t ,
100- fooTx .QueryRow ("SELECT count(*) FROM t" ).Scan (& count ))
108+ outerTx .QueryRow ("SELECT count(*) FROM t" ).Scan (& count ))
101109 require .Equal (t , 0 , count )
102- conn , err := sqlDB .Conn (ctx )
103- require .NoError (t , err )
104- defer func () { _ = conn .Close () }()
105- // Set a timeout on the SQL operations to ensure that they both
106- // happen in a timely manner.
107- grantRevokeTimeout , cancel := context .WithTimeout (
108- ctx , testutils .DefaultSucceedsSoonDuration ,
109- )
110- defer cancel ()
111-
112- _ , err = conn .ExecContext (grantRevokeTimeout , "GRANT foo TO baz;" )
113- require .NoError (t , err )
114- _ , err = conn .ExecContext (grantRevokeTimeout , "REVOKE bar FROM baz;" )
115- require .NoError (t , err )
116110
117- // Ensure the transaction we held open commits without issue.
118- require .NoError (t , fooTx .Commit ())
119- })
120-
121- t .Run ("session variable prevents waiting during CREATE and DROP role" , func (t * testing.T ) {
122- fooConn , err := fooDB .Conn (ctx )
123- require .NoError (t , err )
124- defer func () { _ = fooConn .Close () }()
125- _ , err = fooConn .ExecContext (ctx , "SET allow_role_memberships_to_change_during_transaction = true;" )
126- require .NoError (t , err )
127- fooTx , err := fooConn .BeginTx (ctx , nil )
128- require .NoError (t , err )
129- // We need to use show roles because that access the system.users table.
130- _ , err = fooTx .Exec ("SHOW ROLES" )
111+ // the first user operation shouldn't block
112+ _ , err = sqlDB .Exec ("GRANT biz TO bar;" )
131113 require .NoError (t , err )
132114
133- conn , err := sqlDB .Conn (ctx )
134- require .NoError (t , err )
135- defer func () { _ = conn .Close () }()
136- // Set a timeout on the SQL operations to ensure that they both
137- // happen in a timely manner.
138- grantRevokeTimeout , cancel := context .WithTimeout (
139- ctx , testutils .DefaultSucceedsSoonDuration ,
140- )
141- defer cancel ()
142-
143- _ , err = conn .ExecContext (grantRevokeTimeout , "CREATE ROLE new_role;" )
144- require .NoError (t , err )
145- _ , err = conn .ExecContext (grantRevokeTimeout , "DROP ROLE new_role;" )
115+ // nor should the second
116+ _ , err = sqlDB .Exec ("GRANT baz TO bar;" )
146117 require .NoError (t , err )
147118
148- // Ensure the transaction we held open commits without issue.
149- require .NoError (t , fooTx .Commit ())
119+ require .NoError (t , outerTx .Commit ())
150120 })
151-
152121}
0 commit comments