Skip to content

Commit

Permalink
Changes after self review and local testing
Browse files Browse the repository at this point in the history 
Signed-off-by: Matt Lord <mattalord@gmail.com>
  • Loading branch information
@mattlord
mattlord committed Oct 4, 2023
1 parent bcb37fd commit 049b779
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 58 deletions.
55 changes: 28 additions & 27 deletions go/cmd/vtctldclient/command/vreplication/lookupvindex/lookupvindex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,18 @@ var (
}

baseOptions = struct {
// This is where the vindex target table and VReplicaiton
// This is where the vindex lookup table and VReplicaiton
// workflow will be created.
TableKeyspace string
// This will also be the name of the VReplication workflow.
// This will be the name of the Lookup Vindex and the name
// of the VReplication workflow.
Name string
Vschema *vschemapb.Keyspace
}{}

// base is the base command for all actions related to Lookup Vindexes.
base = &cobra.Command{
Use: "LookupVindex --keyspace <keyspace> [command] [<vindex spec>]",
Use: "LookupVindex --name <name> --table-keyspace <keyspace> [command] [command-flags]",
Short: "Perform commands related to creating, backfilling, and externalizing Lookup Vindexes using VReplication workflows.",
DisableFlagsInUseLine: true,
Aliases: []string{"lookupvindex"},
Expand All @@ -61,11 +62,11 @@ var (
TableOwner string
TableOwnerColumns []string
TableName string
TableKeyspace string
TableVindexType string
Cells []string
TabletTypes []topodatapb.TabletType
TabletTypesInPreferenceOrder bool
IgnoreNulls bool
ContinueAfterCopyWithOwner bool
}{}

Expand All @@ -74,17 +75,21 @@ var (
}{}

parseAndValidateCreate = func(cmd *cobra.Command, args []string) error {
if createOptions.TableKeyspace == "" {
createOptions.TableKeyspace = createOptions.Keyspace
if createOptions.TableName == "" { // Use vindex name
createOptions.TableName = baseOptions.Name
}
if !strings.Contains(createOptions.Type, "lookup") {
return fmt.Errorf("vindex type must be a lookup vindex")
}
baseOptions.Vschema = &vschemapb.Keyspace{
Vindexes: map[string]*vschemapb.Vindex{
baseOptions.Name: {
Type: createOptions.Type,
Params: map[string]string{
"table": createOptions.TableKeyspace + "." + createOptions.TableName,
"from": strings.Join(createOptions.TableOwnerColumns, ","),
"to": "keyspace_id",
"table": baseOptions.TableKeyspace + "." + createOptions.TableName,
"from": strings.Join(createOptions.TableOwnerColumns, ","),
"to": "keyspace_id",
"ignore_nulls": fmt.Sprintf("%t", createOptions.IgnoreNulls),
},
Owner: createOptions.TableOwner,
},
Expand All @@ -110,11 +115,6 @@ var (
},
},
}
// If this is not specified then we'll later use the default for the
// table-owner-columns.
if createOptions.TableVindexType != "" {
baseOptions.Vschema.Tables[createOptions.TableName].ColumnVindexes[0].Name = createOptions.TableVindexType
}

// VReplication specific flags.
ttFlag := cmd.Flags().Lookup("tablet-types")
Expand All @@ -134,7 +134,7 @@ var (
cancel = &cobra.Command{
Use: "cancel",
Short: "Cancel the VReplication workflow that backfills the Lookup Vindex.",
Example: `vtctldclient --server localhost:15999 LookupVindex --keyspace customer cancel '{"sharded":true,"vindexes":{"corder_lookup":{"type":"consistent_lookup_unique","params":{"table":"customer.corder_lookup","from":"sku","to":"keyspace_id"},"owner":"corder"}},"tables":{"corder":{"column_vindexes":[{"column":"sku","name":"corder_lookup"}]}}}'`,
Example: `vtctldclient --server localhost:15999 LookupVindex --name corder_lookup_vdx --table-keyspace customer cancel`,
SilenceUsage: true,
DisableFlagsInUseLine: true,
Aliases: []string{"Cancel"},
Expand All @@ -145,8 +145,8 @@ var (
// create makes a LookupVindexCreate call to a vtctld.
create = &cobra.Command{
Use: "create",
Short: "Create the Lookup Vindex in the specified keyspace and backfill it with a VReplication workflow using the provided Vindex specification.",
Example: `vtctldclient --server localhost:15999 LookupVindex --keyspace customer --name corder_lookup_vdx --type consistent_lookup_unique --table-owner corder --table-owner-columns sku --table-keyspace customer --table-name corder_lookup_tbl --table-vindex-type=unicode_loose_xxhash`,
Short: "Create the Lookup Vindex in the specified keyspace and backfill it with a VReplication workflow.",
Example: `vtctldclient --server localhost:15999 LookupVindex --name corder_lookup_vdx --table-keyspace customer create --keyspace customer --type consistent_lookup_unique --table-owner corder --table-owner-columns sku --table-name corder_lookup_tbl --table-vindex-type=unicode_loose_xxhash`,
SilenceUsage: true,
DisableFlagsInUseLine: true,
Aliases: []string{"Create"},
Expand All @@ -159,7 +159,7 @@ var (
externalize = &cobra.Command{
Use: "externalize",
Short: "Externalize the Lookup Vindex. If the Vindex has an owner the VReplication workflow will also be deleted.",
Example: `vtctldclient --server localhost:15999 LookupVindex --keyspace customer externalize '{"sharded":true,"vindexes":{"corder_lookup":{"type":"consistent_lookup_unique","params":{"table":"customer.corder_lookup","from":"sku","to":"keyspace_id"},"owner":"corder"}},"tables":{"corder":{"column_vindexes":[{"column":"sku","name":"corder_lookup"}]}}}'`,
Example: `vtctldclient --server localhost:15999 LookupVindex --name corder_lookup_vdx --table-keyspace customer externalize`,
SilenceUsage: true,
DisableFlagsInUseLine: true,
Aliases: []string{"Externalize"},
Expand All @@ -171,7 +171,7 @@ var (
show = &cobra.Command{
Use: "show",
Short: "Show the status of the VReplication workflow that backfills the Lookup Vindex.",
Example: `vtctldclient --server localhost:15999 LookupVindex --keyspace customer show '{"sharded":true,"vindexes":{"corder_lookup":{"type":"consistent_lookup_unique","params":{"table":"customer.corder_lookup","from":"sku","to":"keyspace_id"},"owner":"corder"}},"tables":{"corder":{"column_vindexes":[{"column":"sku","name":"corder_lookup"}]}}}'`,
Example: `vtctldclient --server localhost:15999 LookupVindex --name corder_lookup_vdx --table-keyspace customer show`,
SilenceUsage: true,
DisableFlagsInUseLine: true,
Aliases: []string{"Show"},
Expand Down Expand Up @@ -205,7 +205,7 @@ func commandCreate(cmd *cobra.Command, args []string) error {

_, err := common.GetClient().LookupVindexCreate(common.GetCommandCtx(), &vtctldatapb.LookupVindexCreateRequest{
Workflow: baseOptions.Name,
Keyspace: baseOptions.TableKeyspace,
Keyspace: createOptions.Keyspace,
Vindex: baseOptions.Vschema,
Cells: createOptions.Cells,
TabletTypes: createOptions.TabletTypes,
Expand All @@ -231,8 +231,9 @@ func commandExternalize(cmd *cobra.Command, args []string) error {
cli.FinishedParsing(cmd)

resp, err := common.GetClient().LookupVindexExternalize(common.GetCommandCtx(), &vtctldatapb.LookupVindexExternalizeRequest{
Keyspace: externalizeOptions.Keyspace,
Name: baseOptions.Name, // The name of the workflow and lookup vindex.
Keyspace: externalizeOptions.Keyspace,
// The name of the workflow and lookup vindex.
Name: baseOptions.Name,
TargetKeyspace: baseOptions.TableKeyspace,
})

Expand Down Expand Up @@ -274,28 +275,28 @@ func commandShow(cmd *cobra.Command, args []string) error {
func registerCommands(root *cobra.Command) {
base.PersistentFlags().StringVar(&baseOptions.Name, "name", "", "The name of the Lookup Vindex to create. This will also be the name of the VReplication workflow created to backfill the Lookup Vindex.")
base.MarkPersistentFlagRequired("name")
base.PersistentFlags().StringVar(&baseOptions.TableKeyspace, "table-keyspace", "", "The keyspace to create the lookup table in. This is also where the VReplication workflow created to backfill the Lookup Vindex will be created.")
base.PersistentFlags().StringVar(&baseOptions.TableKeyspace, "table-keyspace", "", "The keyspace to create the lookup table in. This is also where the VReplication workflow is created to backfill the Lookup Vindex.")
base.MarkPersistentFlagRequired("table-keyspace")
root.AddCommand(base)

// This will create the lookup vindex in the specified keyspace
// and setup a VReplication workflow to backfill its target table.
// and setup a VReplication workflow to backfill its lookup table.
// vtctldclient --server localhost:15999 LookupVindex --keyspace customer --name corder_lookup_vdx --type consistent_lookup_unique --table-owner corder --table-owner-columns sku --table-keyspace customer --table-name corder_lookup_tbl --table-vindex-type=unicode_loose_xxhash`,
create.Flags().StringVar(&createOptions.Keyspace, "keyspace", "", "The keyspace to create the Lookup Vindex in. This is also where the table-owner must exist.")
create.MarkFlagRequired("keyspace")
create.Flags().StringVar(&createOptions.Type, "type", "", "The type of Lookup Vindex to create.")
create.MarkFlagRequired("type")
create.Flags().StringVar(&createOptions.TableOwner, "table-owner", "", "The table holding the data which we should use to backfill the Lookup Vindex. This must exist in the same keyspace as the Lookup Vindex.")
create.MarkFlagRequired("table-owner")
create.Flags().StringSliceVar(&createOptions.TableOwnerColumns, "table-owner-columns", nil, "The columns to read from the owner table. These will be used to generate the hash which gets stored as the keyspace_id value in the lookup table.")
create.Flags().StringSliceVar(&createOptions.TableOwnerColumns, "table-owner-columns", nil, "The columns to read from the owner table. These will be used to build the hash which gets stored as the keyspace_id value in the lookup table.")
create.MarkFlagRequired("table-owner-columns")
create.Flags().StringVar(&createOptions.TableName, "table-name", "", "The name of the lookup table. If not specified, then it will be created using the same name as the Lookup Vindex.")
create.Flags().StringVar(&createOptions.TableKeyspace, "table-keyspace", "", "The keyspace to create the lookup table in. If not specified, then it will be created in the same keyspace as the Lookup Vindex.")
create.Flags().StringVar(&createOptions.TableVindexType, "table-vindex-type", "", "The primary vindex name/type to use for lookup table, if the table-keyspace is sharded. This must match the name of a vindex defined in the table-keyspace. If no value is provided then the default type will be use based on the table-owner-columns types.")
create.Flags().BoolVar(&createOptions.IgnoreNulls, "ignore-nulls", false, "Do not add records in the lookup table if any of the owner table's 'from' fields are NULL.")
create.Flags().BoolVar(&createOptions.ContinueAfterCopyWithOwner, "continue-after-copy-with-owner", true, "Vindex will continue materialization after copy when an owner is provided.")
// VReplication specific flags.
create.Flags().StringSliceVar(&createOptions.Cells, "cells", nil, "Cells to look in for source tablets to replicate from.")
create.Flags().Var((*topoprotopb.TabletTypeListFlag)(&createOptions.TabletTypes), "tablet-types", "Source tablet types to replicate from.")
create.Flags().BoolVar(&createOptions.ContinueAfterCopyWithOwner, "continue-after-copy-with-owner", true, "Vindex will continue materialization after copy when an owner is provided")
create.Flags().BoolVar(&createOptions.TabletTypesInPreferenceOrder, "tablet-types-in-preference-order", true, "When performing source tablet selection, look for candidates in the type order as they are listed in the tablet-types flag.")
base.AddCommand(create)

Expand Down
26 changes: 0 additions & 26 deletions go/test/endtoend/vreplication/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,32 +99,6 @@ create table loadtest (id int, name varchar(256), primary key(id), key(name));
}
}
}
`

createLookupVindexVSchema = `
{
"sharded": true,
"vindexes": {
"customer_name_keyspace_id": {
"type": "consistent_lookup",
"params": {
"table": "product.customer_name_keyspace_id",
"from": "name,cid",
"to": "keyspace_id",
"ignore_nulls": "true"
},
"owner": "customer"
}
},
"tables": {
"customer": {
"column_vindexes": [{
"columns": ["name", "cid"],
"name": "customer_name_keyspace_id"
}]
}
}
}
`

customerSchema = ""
Expand Down
6 changes: 4 additions & 2 deletions go/test/endtoend/vreplication/vreplication_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ func TestVreplicationCopyThrottling(t *testing.T) {
func TestBasicVreplicationWorkflow(t *testing.T) {
sourceKsOpts["DBTypeVersion"] = "mysql-8.0"
targetKsOpts["DBTypeVersion"] = "mysql-8.0"
//testBasicVreplicationWorkflow(t, "noblob")
testBasicVreplicationWorkflow(t, "noblob")
}

Expand Down Expand Up @@ -386,7 +387,8 @@ func testVreplicationWorkflows(t *testing.T, limited bool, binlogRowImage string
_, err = vtgateConn.ExecuteFetch(insert, -1, false)
require.NoError(t, err, "error executing %q: %v", insert, err)

err = vc.VtctldClient.ExecuteCommand("LookupVindex", "--keyspace=customer", "create", "--tablet-types=PRIMARY", createLookupVindexVSchema)
err = vc.VtctldClient.ExecuteCommand("LookupVindex", "--name=corder_lookup", "--table-keyspace=product", "create", "--keyspace=customer",
"--type=consistent_lookup", "--table-owner=customer", "--table-name=customer", "--table-owner-columns=name,cid", "--ignore-nulls", "--tablet-types=PRIMARY")
require.NoError(t, err, "error executing LookupVindex create: %v", err)
waitForWorkflowState(t, vc, "product.customer_name_keyspace_id_vdx", binlogdatapb.VReplicationWorkflowState_Running.String())
waitForRowCount(t, vtgateConn, "product", "customer_name_keyspace_id", int(rows))
Expand All @@ -396,7 +398,7 @@ func testVreplicationWorkflows(t *testing.T, limited bool, binlogRowImage string
require.NotNil(t, vdx, "lookup vindex customer_name_keyspace_id not found")
require.Equal(t, "true", vdx.Get("params.write_only").String(), "expected write_only parameter to be true")

err = vc.VtctldClient.ExecuteCommand("LookupVindex", "--keyspace=customer", "externalize", createLookupVindexVSchema)
err = vc.VtctldClient.ExecuteCommand("LookupVindex", "--name=corder_lookup", "--table-keyspace=product", "externalize")
require.NoError(t, err, "error executing LookupVindex externalize: %v", err)
customerVSchema, err = vc.VtctldClient.ExecuteCommandWithOutput("GetVSchema", "customer")
require.NoError(t, err, "error executing GetVSchema: %v", err)
Expand Down
7 changes: 4 additions & 3 deletions go/vt/vtctl/workflow/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3305,7 +3305,7 @@ func (s *Server) prepareCreateLookup(ctx context.Context, workflow, keyspace str

// Validate input vindex.
if len(specs.Vindexes) != 1 {
return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "only one vindex must be specified in the specs: %v", specs.Vindexes)
return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "only one vindex must be specified: %v", specs.Vindexes)
}
vindexName = maps.Keys(specs.Vindexes)[0]
vindex = maps.Values(specs.Vindexes)[0]
Expand Down Expand Up @@ -3349,7 +3349,7 @@ func (s *Server) prepareCreateLookup(ctx context.Context, workflow, keyspace str

// Validate input table.
if len(specs.Tables) < 1 || len(specs.Tables) > 2 {
return nil, nil, nil, fmt.Errorf("one or two tables must be specified in the specs: %v", specs.Tables)
return nil, nil, nil, fmt.Errorf("one or two tables must be specified: %v", specs.Tables)
}
// Loop executes once or twice.
for tableName, table := range specs.Tables {
Expand Down Expand Up @@ -3531,7 +3531,8 @@ func (s *Server) prepareCreateLookup(ctx context.Context, workflow, keyspace str
// Update targetVSchema.
targetTable := specs.Tables[targetTableName]
if targetVSchema.Sharded {
// Choose a primary vindex type for the target table based on the source definition.
// Choose a primary vindex type for the target table based on the source
// definition if one was not explicitly specified.
var targetVindexType string
var targetVindex *vschemapb.Vindex
for _, field := range tableSchema.TableDefinitions[0].Fields {
Expand Down

0 comments on commit 049b779

Please sign in to comment.