Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VReplication: Replace most usage of SimulatedNulls #16734

Merged
merged 7 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions go/cmd/vtctldclient/command/vreplication/common/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ import (
"sort"
"strings"

"vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

"github.com/spf13/cobra"

"vitess.io/vitess/go/cmd/vtctldclient/cli"
"vitess.io/vitess/go/textutil"
"vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
Expand Down Expand Up @@ -143,9 +142,8 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
Workflow: workflowUpdateOptions.Workflow,
Cells: textutil.SimulatedNullStringSlice,
TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
OnDdl: binlogdatapb.OnDDLAction(textutil.SimulatedNullInt),
State: state,
TabletTypes: textutil.SimulatedNullTabletTypeSlice,
State: &state,
},
}

Expand Down
6 changes: 2 additions & 4 deletions go/cmd/vtctldclient/command/vreplication/workflow/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
)

Expand Down Expand Up @@ -79,9 +78,8 @@ func commandUpdateState(cmd *cobra.Command, args []string) error {
TabletRequest: &tabletmanagerdatapb.UpdateVReplicationWorkflowRequest{
Workflow: baseOptions.Workflow,
Cells: textutil.SimulatedNullStringSlice,
TabletTypes: []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)},
OnDdl: binlogdatapb.OnDDLAction(textutil.SimulatedNullInt),
State: state,
TabletTypes: textutil.SimulatedNullTabletTypeSlice,
State: &state,
},
}

Expand Down
25 changes: 11 additions & 14 deletions go/cmd/vtctldclient/command/vreplication/workflow/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"vitess.io/vitess/go/cmd/vtctldclient/cli"
"vitess.io/vitess/go/cmd/vtctldclient/command/vreplication/common"
"vitess.io/vitess/go/ptr"
"vitess.io/vitess/go/textutil"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
Expand Down Expand Up @@ -65,14 +66,14 @@ var (
}
changes = true
} else {
updateOptions.TabletTypes = []topodatapb.TabletType{topodatapb.TabletType(textutil.SimulatedNullInt)}
updateOptions.TabletTypes = textutil.SimulatedNullTabletTypeSlice
}
if cmd.Flags().Lookup("on-ddl").Changed { // Validate the provided value
changes = true
if _, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; !ok {
return fmt.Errorf("invalid on-ddl value: %s", updateOptions.OnDDL)
}
} // Simulated NULL will need to be handled in command
}
if !changes {
return fmt.Errorf("no configuration options specified to update")
}
Expand All @@ -85,15 +86,6 @@ var (
func commandUpdate(cmd *cobra.Command, args []string) error {
cli.FinishedParsing(cmd)

// We've already validated any provided value, if one WAS provided.
// Now we need to do the mapping from the string representation to
// the enum value.
onddl := int32(textutil.SimulatedNullInt) // Simulated NULL when no value provided
if val, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; ok {
onddl = val
}

// Simulated NULL when no value is provided.
tsp := tabletmanagerdatapb.TabletSelectionPreference_UNKNOWN
if cmd.Flags().Lookup("tablet-types-in-order").Changed {
if updateOptions.TabletTypesInPreferenceOrder {
Expand All @@ -109,12 +101,17 @@ func commandUpdate(cmd *cobra.Command, args []string) error {
Workflow: baseOptions.Workflow,
Cells: updateOptions.Cells,
TabletTypes: updateOptions.TabletTypes,
TabletSelectionPreference: tsp,
OnDdl: binlogdatapb.OnDDLAction(onddl),
State: binlogdatapb.VReplicationWorkflowState(textutil.SimulatedNullInt), // We don't allow changing this in the client command
TabletSelectionPreference: &tsp,
},
}

// We've already validated any provided value, if one WAS provided.
// Now we need to do the mapping from the string representation to
// the enum value.
if val, ok := binlogdatapb.OnDDLAction_value[strings.ToUpper(updateOptions.OnDDL)]; ok {
req.TabletRequest.OnDdl = ptr.Of(binlogdatapb.OnDDLAction(val))
}

resp, err := common.GetClient().WorkflowUpdate(common.GetCommandCtx(), req)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions go/test/endtoend/vreplication/vdiff2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"

"vitess.io/vitess/go/ptr"
"vitess.io/vitess/go/test/endtoend/cluster"
"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/sqlparser"
Expand Down Expand Up @@ -370,7 +371,6 @@ func testCLIErrors(t *testing.T, ksWorkflow, cells string) {
// testCLIFlagHandling tests that the vtctldclient CLI flags are handled correctly
// from vtctldclient->vtctld->vttablet->mysqld.
func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell) {
false := false
expectedOptions := &tabletmanagerdatapb.VDiffOptions{
CoreOptions: &tabletmanagerdatapb.VDiffCoreOptions{
MaxRows: 999,
Expand All @@ -379,7 +379,7 @@ func testCLIFlagHandling(t *testing.T, targetKs, workflowName string, cell *Cell
UpdateTableStats: true,
TimeoutSeconds: 60,
MaxDiffSeconds: 333,
AutoStart: &false,
AutoStart: ptr.Of(false),
},
PickerOptions: &tabletmanagerdatapb.VDiffPickerOptions{
SourceCell: "zone1,zone2,zone3,zonefoosource",
Expand Down
30 changes: 8 additions & 22 deletions go/textutil/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"vitess.io/vitess/go/sqltypes"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
)

Expand All @@ -37,10 +36,10 @@ const (
)

var (
delimitedListRegexp = regexp.MustCompile(`[ ,;]+`)
SimulatedNullString = sqltypes.NULL.String()
SimulatedNullStringSlice = []string{sqltypes.NULL.String()}
SimulatedNullInt = -1
delimitedListRegexp = regexp.MustCompile(`[ ,;]+`)
SimulatedNullStringSlice = []string{sqltypes.NULL.String()}
SimulatedNullTabletTypeSlice = []topodatapb.TabletType{topodatapb.TabletType(SimulatedNullInt)}
SimulatedNullInt = -1
)

// SplitDelimitedList splits a given string by comma, semi-colon or space, and returns non-empty strings
Expand Down Expand Up @@ -91,29 +90,16 @@ func SingleWordCamel(w string) string {
return strings.ToUpper(w[0:1]) + strings.ToLower(w[1:])
}

// ValueIsSimulatedNull returns true if the value represents
// a NULL or unknown/unspecified value. This is used to
// distinguish between a zero value / default and a user
// provided value that is equivalent (e.g. an empty string
// or slice).
// ValueIsSimulatedNull returns true if the slice value represents
// a NULL or unknown/unspecified value. This is used to distinguish
// between a zero value empty slice and a user provided value of an
// empty slice.
func ValueIsSimulatedNull(val any) bool {
switch cval := val.(type) {
case string:
return cval == SimulatedNullString
case []string:
return len(cval) == 1 && cval[0] == sqltypes.NULL.String()
case binlogdatapb.OnDDLAction:
return int32(cval) == int32(SimulatedNullInt)
case int:
return cval == SimulatedNullInt
case int32:
return int32(cval) == int32(SimulatedNullInt)
case int64:
return int64(cval) == int64(SimulatedNullInt)
case []topodatapb.TabletType:
return len(cval) == 1 && cval[0] == topodatapb.TabletType(SimulatedNullInt)
case binlogdatapb.VReplicationWorkflowState:
return int32(cval) == int32(SimulatedNullInt)
default:
return false
}
Expand Down
41 changes: 14 additions & 27 deletions go/textutil/strings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/sqltypes"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
)
Expand Down Expand Up @@ -131,53 +133,38 @@ func TestValueIsSimulatedNull(t *testing.T) {
val: "test",
isNull: false,
},
{
name: "case string true",
val: SimulatedNullString,
isNull: true,
},
{
name: "case []string true",
val: []string{SimulatedNullString},
val: []string{sqltypes.NULL.String()},
isNull: true,
},
{
name: "case []string false",
val: []string{SimulatedNullString, SimulatedNullString},
val: []string{sqltypes.NULL.String(), sqltypes.NULL.String()},
isNull: false,
},
{
name: "case binlogdatapb.OnDDLAction true",
val: binlogdatapb.OnDDLAction(SimulatedNullInt),
isNull: true,
},
{
name: "case int true",
val: SimulatedNullInt,
isNull: true,
},
{
name: "case int32 true",
val: int32(SimulatedNullInt),
isNull: true,
name: "case binlogdatapb.OnDDLAction exec",
val: binlogdatapb.OnDDLAction_EXEC,
isNull: false,
},
{
name: "case int64 true",
val: int64(SimulatedNullInt),
isNull: true,
name: "case int false",
val: 1,
isNull: false,
},
{
name: "case []topodatapb.TabletType true",
val: []topodatapb.TabletType{topodatapb.TabletType(SimulatedNullInt)},
isNull: true,
},
{
name: "case binlogdatapb.VReplicationWorkflowState true",
val: binlogdatapb.VReplicationWorkflowState(SimulatedNullInt),
isNull: true,
name: "case binlogdatapb.VReplicationWorkflowState running",
val: binlogdatapb.VReplicationWorkflowState_Running,
isNull: false,
},
{
name: "case default",
name: "case float false",
val: float64(1),
isNull: false,
},
Expand Down
Loading
Loading