Skip to content

Commit

Permalink
Merge cockroachdb#93755 cockroachdb#94695
Browse files Browse the repository at this point in the history 
93755: streamingest: c2c UX fixes r=lidorcarmel a=lidorcarmel

CREATE TENANT FROM REPLICATION: currently has an output (the job ids of the producer and consumer). This PR removes the output.

ALTER TENANT PAUSE/RESUME: currently prints the job id, and this PR removes the output.

ALTER TENANT COMPLETE: currently prints the job id. This PR changes the output to be the cutover timestamp. The rational is that the user may not know the cutover time because LATEST or NOW() etc. was used.

Fixes: cockroachdb#94706

Epic: CRDB-18752

Release note: None

94695: sql,descs,*: rewrite by-ID and by-name descriptor lookups r=postamar a=postamar

This commit replaces all Get(Mutable|Immutable)*By(ID|Name) methods on the
descs.Collection and their usages with the recently-introduced
By(ID|Name)Getter methods.

While this commit shouldn't fundamentally change anything as far as
functionality is concerned, it isn't strictly-speaking a refactor
because the default behaviors have been changed slightly to make them
consistent accross similar kinds of lookups. Here's an illustrative
example: previously, GetImmutableDatabaseByID and GetImmutableSchemaByID
would honor the Required flag while the other GetImmutable*ByID methods
would ignore it and return an error when the descriptor is not found,
presently all immutable by-ID lookups always return an error when the
descriptor is not found.

The new API does away with the tree.(Common|Object)LookupFlags and these
have been cleaned up because now only the resolver uses them.

Fixes cockroachdb#87753.

Release note: None

Co-authored-by: Lidor Carmel <[email protected]>
Co-authored-by: Marius Posta <[email protected]>
  • Loading branch information
@lidorcarmel
3 people committed Jan 6, 2023
3 parents c1b0124 + 01d22ea + af6a72a commit 39ee43d
Show file tree
Hide file tree
Showing 170 changed files with 1,062 additions and 2,403 deletions.
2 changes: 1 addition & 1 deletion pkg/bench/rttanalysis/testdata/benchmark_expectations
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ exp,benchmark
5,GenerateObjects/generate_50000_tables
5,GenerateObjects/generate_1000_tables_-_this_test_should_use_the_same_number_of_RTTs_as_for_10_tables
5,GenerateObjects/generate_10_tables
10,GenerateObjects/generate_100_tables_from_template
12,GenerateObjects/generate_100_tables_from_template
16,GenerateObjects/generate_10x10_schemas_and_tables_in_existing_db
9,Grant/grant_all_on_1_table
9,Grant/grant_all_on_2_tables
Expand Down
64 changes: 24 additions & 40 deletions pkg/ccl/backupccl/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10448,34 +10448,30 @@ $$;
udfID, err := strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 104, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 106, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 107, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 108, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 110, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 111, int(fnDesc.GetID()))
require.Equal(t, 104, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10504,34 +10500,30 @@ $$;
udfID, err = strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1_new", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1_new")
require.NoError(t, err)
require.Equal(t, 112, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 114, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1_new", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 115, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1_new", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 116, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1_new", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 118, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 119, int(fnDesc.GetID()))
require.Equal(t, 112, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10595,34 +10587,30 @@ $$;
udfID, err := strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 104, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 106, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 107, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 108, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 110, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 111, int(fnDesc.GetID()))
require.Equal(t, 104, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10653,34 +10641,30 @@ $$;
udfID, err = strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, tgtServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 107, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 125, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 126, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 127, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 129, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 130, int(fnDesc.GetID()))
require.Equal(t, 107, int(fnDesc.GetParentID()))
Expand Down
73 changes: 16 additions & 57 deletions pkg/ccl/backupccl/restore_job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ func createImportingDescriptors(
// Write the updated databases.
for dbID, schemas := range existingDBsWithNewSchemas {
log.Infof(ctx, "writing %d schema entries to database %d", len(schemas), dbID)
desc, err := descsCol.GetMutableDescriptorByID(ctx, txn, dbID)
desc, err := descsCol.MutableByID(txn).Desc(ctx, dbID)
if err != nil {
return err
}
Expand All @@ -1093,12 +1093,7 @@ func createImportingDescriptors(
// to the new tables being restored.
for _, table := range mutableTables {
// Collect all types used by this table.
_, dbDesc, err := descsCol.GetImmutableDatabaseByID(
ctx, txn, table.GetParentID(), tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
})
dbDesc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, table.GetParentID())
if err != nil {
return err
}
Expand All @@ -1120,7 +1115,7 @@ func createImportingDescriptors(
continue
}
// Otherwise, add a backreference to this table.
typDesc, err := descsCol.GetMutableTypeVersionByID(ctx, txn, id)
typDesc, err := descsCol.MutableByID(txn).Type(ctx, id)
if err != nil {
return err
}
Expand All @@ -1144,16 +1139,7 @@ func createImportingDescriptors(
if details.DescriptorCoverage != tree.AllDescriptors {
for _, table := range tableDescs {
if lc := table.GetLocalityConfig(); lc != nil {
_, desc, err := descsCol.GetImmutableDatabaseByID(
ctx,
txn,
table.ParentID,
tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
},
)
desc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, table.ParentID)
if err != nil {
return err
}
Expand All @@ -1163,7 +1149,7 @@ func createImportingDescriptors(
table.ID, table.ParentID)
}

mutTable, err := descsCol.GetMutableTableVersionByID(ctx, table.GetID(), txn)
mutTable, err := descsCol.MutableByID(txn).Table(ctx, table.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2242,7 +2228,7 @@ func prefetchDescriptors(
// and we're going to write them to KV very soon as part of a
// single batch).
ids := allDescIDs.Ordered()
got, err := descsCol.GetMutableDescriptorsByID(ctx, txn, ids...)
got, err := descsCol.MutableByID(txn).Descs(ctx, ids)
if err != nil {
return nstree.Catalog{}, errors.Wrap(err, "prefetch descriptors")
}
Expand Down Expand Up @@ -2374,7 +2360,7 @@ func (r *restoreResumer) dropDescriptors(
mutableTables := make([]*tabledesc.Mutable, len(details.TableDescs))
for i := range details.TableDescs {
var err error
mutableTables[i], err = descsCol.GetMutableTableVersionByID(ctx, details.TableDescs[i].ID, txn)
mutableTables[i], err = descsCol.MutableByID(txn).Table(ctx, details.TableDescs[i].ID)
if err != nil {
return err
}
Expand Down Expand Up @@ -2453,12 +2439,7 @@ func (r *restoreResumer) dropDescriptors(
// TypeDescriptors don't have a GC job process, so we can just write them
// as dropped here.
typDesc := details.TypeDescs[i]
mutType, err := descsCol.GetMutableTypeByID(ctx, txn, typDesc.ID, tree.ObjectLookupFlags{
CommonLookupFlags: tree.CommonLookupFlags{
AvoidLeased: true,
IncludeOffline: true,
},
})
mutType, err := descsCol.MutableByID(txn).Type(ctx, typDesc.ID)
if err != nil {
return err
}
Expand All @@ -2474,12 +2455,7 @@ func (r *restoreResumer) dropDescriptors(

for i := range details.FunctionDescs {
fnDesc := details.FunctionDescs[i]
mutFn, err := descsCol.GetMutableFunctionByID(ctx, txn, fnDesc.ID, tree.ObjectLookupFlags{
CommonLookupFlags: tree.CommonLookupFlags{
AvoidLeased: true,
IncludeOffline: true,
},
})
mutFn, err := descsCol.MutableByID(txn).Function(ctx, fnDesc.ID)
if err != nil {
return err
}
Expand Down Expand Up @@ -2551,13 +2527,13 @@ func (r *restoreResumer) dropDescriptors(
continue
}

mutSchema, err := descsCol.GetMutableDescriptorByID(ctx, txn, schemaDesc.GetID())
mutSchema, err := descsCol.MutableByID(txn).Desc(ctx, schemaDesc.GetID())
if err != nil {
return err
}
entry, hasEntry := dbsWithDeletedSchemas[schemaDesc.GetParentID()]
if !hasEntry {
mutParent, err := descsCol.GetMutableDescriptorByID(ctx, txn, schemaDesc.GetParentID())
mutParent, err := descsCol.MutableByID(txn).Desc(ctx, schemaDesc.GetParentID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2628,7 +2604,7 @@ func (r *restoreResumer) dropDescriptors(
continue
}

db, err := descsCol.GetMutableDescriptorByID(ctx, txn, dbDesc.GetID())
db, err := descsCol.MutableByID(txn).Desc(ctx, dbDesc.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2682,24 +2658,12 @@ func setGCTTLForDroppingTable(
log.VInfof(ctx, 2, "lowering TTL for table %q (%d)", tableToDrop.GetName(), tableToDrop.GetID())
// We get a mutable descriptor here because we are going to construct a
// synthetic descriptor collection in which they are online.
_, dbDesc, err := descsCol.GetImmutableDatabaseByID(ctx, txn, tableToDrop.GetParentID(),
tree.DatabaseLookupFlags{
Required: true,
IncludeOffline: true,
IncludeDropped: true,
AvoidLeased: true,
})
dbDesc, err := descsCol.ByID(txn).Get().Database(ctx, tableToDrop.GetParentID())
if err != nil {
return err
}

schemaDesc, err := descsCol.GetImmutableSchemaByID(ctx, txn, tableToDrop.GetParentSchemaID(),
tree.SchemaLookupFlags{
Required: true,
IncludeDropped: true,
IncludeOffline: true,
AvoidLeased: true,
})
schemaDesc, err := descsCol.ByID(txn).Get().Schema(ctx, tableToDrop.GetParentSchemaID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2758,20 +2722,15 @@ func (r *restoreResumer) removeExistingTypeBackReferences(
return restored, nil
}
// Finally, look it up using the transaction.
typ, err := descsCol.GetMutableTypeVersionByID(ctx, txn, id)
typ, err := descsCol.MutableByID(txn).Type(ctx, id)
if err != nil {
return nil, err
}
existingTypes[typ.GetID()] = typ
return typ, nil
}

_, dbDesc, err := descsCol.GetImmutableDatabaseByID(
ctx, txn, tbl.GetParentID(), tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
})
dbDesc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, tbl.GetParentID())
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 39ee43d

Please sign in to comment.