Skip to content

Commit e1eea7d

Browse files
trunk-io[bot]trunk-io[bot]
authored
sql: replace TestingDisableTableLeases with AcquireFreshestFromStore (#167084)
sql: replace TestingDisableTableLeases with AcquireFreshestFromStore
2 parents 72f5c45 + 005f66b commit e1eea7d

5 files changed

Lines changed: 75 additions & 138 deletions

File tree

‎pkg/sql/crdb_internal_test.go‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -209,19 +209,9 @@ func TestOldBitColumnMetadata(t *testing.T) {
209209
defer log.Scope(t).Close(t)
210210

211211
ctx := context.Background()
212-
s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{
213-
// This test directly manipulates descriptors via KV and relies on
214-
// TestingDisableTableLeases to make those changes immediately visible.
215-
// This is incompatible with external process virtual clusters where
216-
// the lease disable doesn't take effect.
217-
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(166311),
218-
})
212+
s, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
219213
defer s.Stopper().Stop(ctx)
220214

221-
// The descriptor changes made must have an immediate effect
222-
// so disable leases on tables.
223-
defer lease.TestingDisableTableLeases()()
224-
225215
if _, err := sqlDB.Exec(`
226216
CREATE DATABASE t;
227217
CREATE TABLE t.test (k INT);
@@ -268,13 +258,22 @@ CREATE TABLE t.test (k INT);
268258
tableDesc.PrimaryIndex.StoreColumnIDs = append(tableDesc.PrimaryIndex.StoreColumnIDs, col.ID)
269259
tableDesc.PrimaryIndex.StoreColumnNames = append(tableDesc.PrimaryIndex.StoreColumnNames, col.Name)
270260

271-
// Write the modified descriptor.
261+
// Write the modified descriptor. Bump the version so the lease manager
262+
// recognizes the change.
263+
tableDesc.Version++
272264
if err := kvDB.Txn(context.Background(), func(ctx context.Context, txn *kv.Txn) error {
273265
return txn.Put(ctx, catalogkeys.MakeDescMetadataKey(s.Codec(), tableDesc.ID), tableDesc.DescriptorProto())
274266
}); err != nil {
275267
t.Fatal(err)
276268
}
277269

270+
// Force the lease manager to acquire a fresh descriptor from the store,
271+
// since we wrote it directly via KV above.
272+
lm := s.LeaseManager().(*lease.Manager)
273+
if err := lm.AcquireFreshestFromStore(ctx, tableDesc.GetID()); err != nil {
274+
t.Fatal(err)
275+
}
276+
278277
// Read the column metadata from information_schema.
279278
rows, err := sqlDB.Query(`
280279
SELECT column_name, character_maximum_length, numeric_precision, numeric_precision_radix, crdb_sql_type
@@ -431,13 +430,7 @@ func TestInvalidObjects(t *testing.T) {
431430
defer log.Scope(t).Close(t)
432431

433432
ctx := context.Background()
434-
params := base.TestServerArgs{
435-
// This test directly manipulates descriptors via KV and relies on
436-
// TestingDisableTableLeases to make those changes immediately visible.
437-
// This is incompatible with external process virtual clusters where
438-
// the lease disable doesn't take effect.
439-
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(166311),
440-
}
433+
params := base.TestServerArgs{}
441434
params.Knobs = base.TestingKnobs{
442435
Store: &kvserver.StoreTestingKnobs{
443436
DisableMergeQueue: true,
@@ -446,10 +439,6 @@ func TestInvalidObjects(t *testing.T) {
446439
s, sqlDB, _ := serverutils.StartServer(t, params)
447440
defer s.Stopper().Stop(ctx)
448441

449-
// The descriptor changes made must have an immediate effect
450-
// so disable leases on tables.
451-
defer lease.TestingDisableTableLeases()()
452-
453442
tdb := sqlutils.MakeSQLRunner(sqlDB)
454443

455444
// No inconsistency should be found.

‎pkg/sql/delete_preserving_index_test.go‎

Lines changed: 23 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -247,18 +247,9 @@ func TestDeletePreservingIndexEncodingUsesNormalDeletesInDeleteOnly(t *testing.T
247247
defer leaktest.AfterTest(t)()
248248
defer log.Scope(t).Close(t)
249249

250-
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{
251-
// This test directly manipulates descriptors via KV and relies on
252-
// TestingDisableTableLeases to make those changes immediately visible.
253-
// This is incompatible with external process virtual clusters where
254-
// the lease disable doesn't take effect.
255-
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(166311),
256-
})
250+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
257251
defer server.Stopper().Stop(context.Background())
258-
259-
// The descriptor changes made must have an immediate effect
260-
// so disable leases on tables.
261-
defer lease.TestingDisableTableLeases()()
252+
lm := server.LeaseManager().(*lease.Manager)
262253

263254
setupSQL := `
264255
CREATE DATABASE t;
@@ -275,7 +266,7 @@ CREATE UNIQUE INDEX test_index_to_mutate ON t.test (b);
275266
err = mutateIndexByName(kvDB, codec, tableDesc, "test_index_to_mutate", func(idx *descpb.IndexDescriptor) error {
276267
idx.UseDeletePreservingEncoding = true
277268
return nil
278-
}, descpb.DescriptorMutation_DELETE_ONLY)
269+
}, descpb.DescriptorMutation_DELETE_ONLY, lm)
279270
require.NoError(t, err)
280271

281272
_, err = sqlDB.Exec(`INSERT INTO t.test VALUES (1, 1)`)
@@ -286,7 +277,7 @@ CREATE UNIQUE INDEX test_index_to_mutate ON t.test (b);
286277

287278
// Move index to WRITE_ONLY. The following inserts
288279
// are seen by the index and deletes should be preserved.
289-
err = mutateIndexByName(kvDB, codec, tableDesc, "test_index_to_mutate", nil, descpb.DescriptorMutation_WRITE_ONLY)
280+
err = mutateIndexByName(kvDB, codec, tableDesc, "test_index_to_mutate", nil, descpb.DescriptorMutation_WRITE_ONLY, lm)
290281
require.NoError(t, err)
291282

292283
_, err = sqlDB.Exec(`INSERT INTO t.test VALUES (2, 2), (3, 3)`)
@@ -317,18 +308,9 @@ func TestDeletePreservingIndexEncodingWithEmptyValues(t *testing.T) {
317308
defer leaktest.AfterTest(t)()
318309
defer log.Scope(t).Close(t)
319310

320-
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{
321-
// This test directly manipulates descriptors via KV and relies on
322-
// TestingDisableTableLeases to make those changes immediately visible.
323-
// This is incompatible with external process virtual clusters where
324-
// the lease disable doesn't take effect.
325-
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(166311),
326-
})
311+
server, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
327312
defer server.Stopper().Stop(context.Background())
328-
329-
// The descriptor changes made must have an immediate effect
330-
// so disable leases on tables.
331-
defer lease.TestingDisableTableLeases()()
313+
lm := server.LeaseManager().(*lease.Manager)
332314

333315
setupSQL := `
334316
CREATE DATABASE t;
@@ -352,7 +334,7 @@ CREATE UNIQUE INDEX test_index_to_mutate ON t.test (y) STORING (z, a);
352334
idx.StoreColumnIDs = []catid.ColumnID{0x1, 0x3, 0x4}
353335
idx.KeySuffixColumnIDs = nil
354336
return nil
355-
}, descpb.DescriptorMutation_WRITE_ONLY)
337+
}, descpb.DescriptorMutation_WRITE_ONLY, lm)
356338
require.NoError(t, err)
357339
_, err = sqlDB.Exec(`INSERT INTO t.test VALUES (1, 1, 1, 1); DELETE FROM t.test WHERE x = 1;`)
358340
require.NoError(t, err)
@@ -365,6 +347,7 @@ func mutateIndexByName(
365347
index string,
366348
fn func(*descpb.IndexDescriptor) error,
367349
state descpb.DescriptorMutation_State,
350+
leaseManager *lease.Manager,
368351
) error {
369352
idx, err := catalog.MustFindIndexByName(tableDesc, index)
370353
if err != nil {
@@ -395,11 +378,16 @@ func mutateIndexByName(
395378
tableDesc.Mutations[ord] = m
396379
}
397380
tableDesc.Version++
398-
return kvDB.Put(
381+
if err := kvDB.Put(
399382
context.Background(),
400383
catalogkeys.MakeDescMetadataKey(codec, tableDesc.ID),
401384
tableDesc.DescriptorProto(),
402-
)
385+
); err != nil {
386+
return err
387+
}
388+
// Force the lease manager to acquire a fresh descriptor from the store,
389+
// since we wrote it directly via KV above.
390+
return leaseManager.AcquireFreshestFromStore(context.Background(), tableDesc.GetID())
403391
}
404392

405393
type WrappedVersionedValues struct {
@@ -620,15 +608,9 @@ func TestMergeProcessor(t *testing.T) {
620608
}
621609

622610
run := func(t *testing.T, test TestCase) {
623-
server, tdb, kvDB := serverutils.StartServer(t, base.TestServerArgs{
624-
// This test directly manipulates descriptors via KV and relies on
625-
// TestingDisableTableLeases to make those changes immediately visible.
626-
// This is incompatible with external process virtual clusters where
627-
// the lease disable doesn't take effect.
628-
DefaultTestTenant: base.TestDoesNotWorkWithExternalProcessMode(166311),
629-
})
611+
server, tdb, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
630612
defer server.Stopper().Stop(context.Background())
631-
defer lease.TestingDisableTableLeases()()
613+
lm := server.LeaseManager().(*lease.Manager)
632614

633615
// Run the initial setupSQL.
634616
if _, err := tdb.Exec(test.setupSQL); err != nil {
@@ -664,27 +646,27 @@ func TestMergeProcessor(t *testing.T) {
664646
}
665647
}
666648

667-
err := mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_WRITE_ONLY)
649+
err := mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_WRITE_ONLY, lm)
668650
require.NoError(t, err)
669-
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, setUseDeletePreservingEncoding(true), descpb.DescriptorMutation_DELETE_ONLY)
651+
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, setUseDeletePreservingEncoding(true), descpb.DescriptorMutation_DELETE_ONLY, lm)
670652
require.NoError(t, err)
671653

672654
if _, err := tdb.Exec(test.dstDataSQL); err != nil {
673655
t.Fatal(err)
674656
}
675657

676-
err = mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_DELETE_ONLY)
658+
err = mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_DELETE_ONLY, lm)
677659
require.NoError(t, err)
678-
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, nil, descpb.DescriptorMutation_WRITE_ONLY)
660+
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, nil, descpb.DescriptorMutation_WRITE_ONLY, lm)
679661
require.NoError(t, err)
680662

681663
if _, err := tdb.Exec(test.srcDataSQL); err != nil {
682664
t.Fatal(err)
683665
}
684666

685-
err = mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_WRITE_ONLY)
667+
err = mutateIndexByName(kvDB, codec, tableDesc, test.dstIndex, nil, descpb.DescriptorMutation_WRITE_ONLY, lm)
686668
require.NoError(t, err)
687-
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, nil, descpb.DescriptorMutation_DELETE_ONLY)
669+
err = mutateIndexByName(kvDB, codec, tableDesc, test.srcIndex, nil, descpb.DescriptorMutation_DELETE_ONLY, lm)
688670
require.NoError(t, err)
689671

690672
if _, err := tdb.Exec(test.dstDataSQL2); err != nil {

0 commit comments

Comments
 (0)