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.

Sign up for GitHub

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

Jump to bottom

VReplication: Fix VDiff2 DeleteByUUID Query #13255

Merged
merged 3 commits into from
Jun 9, 2023
Merged

VReplication: Fix VDiff2 DeleteByUUID Query #13255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ac8f74
Fix VDiff2 DeleteByUUID Query
mattlord Jun 6, 2023
d5f81be
Add unit test as well
mattlord Jun 7, 2023
cac3548
Minor changes after review
mattlord Jun 9, 2023
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
42 changes: 37 additions & 5 deletions go/test/endtoend/vreplication/vdiff2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"time"

"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"

"vitess.io/vitess/go/test/endtoend/cluster"
"vitess.io/vitess/go/vt/sqlparser"
Expand Down Expand Up @@ -234,16 +235,47 @@ func testCLIErrors(t *testing.T, ksWorkflow, cells string) {

func testDelete(t *testing.T, ksWorkflow, cells string) {
t.Run("Delete", func(t *testing.T) {
// test show verbose too as a side effect
// Let's be sure that we have at least 3 unique VDiffs.
// We have one record in the SHOW output per VDiff, per
// shard. So we want to get a count of the unique VDiffs
// by UUID.
uuidCount := func(uuids []gjson.Result) int64 {
mattlord marked this conversation as resolved.
Show resolved Hide resolved
seen := make(map[string]struct{})
for _, uuid := range uuids {
seen[uuid.String()] = struct{}{}
}
return int64(len(seen))
}
_, output := performVDiff2Action(t, ksWorkflow, cells, "show", "all", false)
initialVDiffCount := uuidCount(gjson.Get(output, "#.UUID").Array())
for ; initialVDiffCount < 3; initialVDiffCount++ {
_, _ = performVDiff2Action(t, ksWorkflow, cells, "create", "", false)
}

// Now let's confirm that we have at least 3 unique VDiffs.
_, output = performVDiff2Action(t, ksWorkflow, cells, "show", "all", false)
require.GreaterOrEqual(t, uuidCount(gjson.Get(output, "#.UUID").Array()), int64(3))
// And that our initial count is what we expect.
require.Equal(t, initialVDiffCount, uuidCount(gjson.Get(output, "#.UUID").Array()))

// Test show last with verbose too as a side effect.
uuid, output := performVDiff2Action(t, ksWorkflow, cells, "show", "last", false, "--verbose")
// only present with --verbose
// The TableSummary is only present with --verbose.
require.Contains(t, output, `"TableSummary":`)

// Now let's delete one of the VDiffs.
_, output = performVDiff2Action(t, ksWorkflow, cells, "delete", uuid, false)
require.Contains(t, output, `"Status": "completed"`)
require.Equal(t, "completed", gjson.Get(output, "Status").String())
// And confirm that our unique VDiff count has only decreased by one.
_, output = performVDiff2Action(t, ksWorkflow, cells, "show", "all", false)
require.Equal(t, initialVDiffCount-1, uuidCount(gjson.Get(output, "#.UUID").Array()))

// Now let's delete all of them.
_, output = performVDiff2Action(t, ksWorkflow, cells, "delete", "all", false)
require.Contains(t, output, `"Status": "completed"`)
require.Equal(t, "completed", gjson.Get(output, "Status").String())
// And finally confirm that we have no more VDiffs.
_, output = performVDiff2Action(t, ksWorkflow, cells, "show", "all", false)
require.Equal(t, "[]\n", output)
require.Equal(t, int64(0), gjson.Get(output, "#").Int())
})
}

Expand Down
27 changes: 13 additions & 14 deletions go/test/endtoend/vreplication/vdiff_helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (
"testing"
"time"

"github.com/buger/jsonparser"
"github.com/stretchr/testify/require"
"github.com/tidwall/gjson"

"vitess.io/vitess/go/sqlescape"
"vitess.io/vitess/go/sqltypes"
Expand Down Expand Up @@ -176,7 +176,7 @@ func performVDiff2Action(t *testing.T, ksWorkflow, cells, action, actionArg stri
log.Infof("vdiff2 output: %+v (err: %+v)", output, err)
if !expectError {
require.Nil(t, err)
uuid, err = jsonparser.GetString([]byte(output), "UUID")
uuid = gjson.Get(output, "UUID").String()
if action != "delete" && !(action == "show" && actionArg == "all") { // a UUID is not required
require.NoError(t, err)
require.NotEmpty(t, uuid)
Expand All @@ -195,19 +195,18 @@ type vdiffInfo struct {
Progress vdiff2.ProgressReport
}

func getVDiffInfo(jsonStr string) *vdiffInfo {
func getVDiffInfo(json string) *vdiffInfo {
var info vdiffInfo
json := []byte(jsonStr)
info.Workflow, _ = jsonparser.GetString(json, "Workflow")
info.Keyspace, _ = jsonparser.GetString(json, "Keyspace")
info.State, _ = jsonparser.GetString(json, "State")
info.Shards, _ = jsonparser.GetString(json, "Shards")
info.RowsCompared, _ = jsonparser.GetInt(json, "RowsCompared")
info.StartedAt, _ = jsonparser.GetString(json, "StartedAt")
info.CompletedAt, _ = jsonparser.GetString(json, "CompletedAt")
info.HasMismatch, _ = jsonparser.GetBoolean(json, "HasMismatch")
info.Progress.Percentage, _ = jsonparser.GetFloat(json, "Progress", "Percentage")
info.Progress.ETA, _ = jsonparser.GetString(json, "Progress", "ETA")
info.Workflow = gjson.Get(json, "Workflow").String()
info.Keyspace = gjson.Get(json, "Keyspace").String()
info.State = gjson.Get(json, "State").String()
info.Shards = gjson.Get(json, "Shards").String()
info.RowsCompared = gjson.Get(json, "RowsCompared").Int()
info.StartedAt = gjson.Get(json, "StartedAt").String()
info.CompletedAt = gjson.Get(json, "CompletedAt").String()
info.HasMismatch = gjson.Get(json, "HasMismatch").Bool()
info.Progress.Percentage = gjson.Get(json, "Progress.Percentage").Float()
info.Progress.ETA = gjson.Get(json, "Progress.ETA").String()
return &info
}

Expand Down
53 changes: 47 additions & 6 deletions go/vt/vttablet/tabletmanager/vdiff/action_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,14 @@ package vdiff

import (
"context"
"fmt"
"reflect"
"testing"
"time"

"github.com/google/uuid"

"vitess.io/vitess/go/sqltypes"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"
Expand All @@ -30,27 +34,64 @@ import (
func TestPerformVDiffAction(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
vdiffenv := newTestVDiffEnv(t)
defer vdiffenv.close()
keyspace := "ks"
workflow := "wf"
uuid := uuid.New().String()
tests := []struct {
name string
vde *Engine
req *tabletmanagerdatapb.VDiffRequest
want *tabletmanagerdatapb.VDiffResponse
wantErr error
name string
vde *Engine
req *tabletmanagerdatapb.VDiffRequest
want *tabletmanagerdatapb.VDiffResponse
expectQueries []string
wantErr error
}{
{
name: "engine not open",
vde: &Engine{isOpen: false},
wantErr: vterrors.New(vtrpcpb.Code_UNAVAILABLE, "vdiff engine is closed"),
},
{
name: "delete by uuid",
req: &tabletmanagerdatapb.VDiffRequest{
Action: string(DeleteAction),
ActionArg: uuid,
},
expectQueries: []string{
fmt.Sprintf(`delete from vd, vdt using _vt.vdiff as vd left join _vt.vdiff_table as vdt on (vd.id = vdt.vdiff_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is same as sqlDeleteVDiffByUUID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I intentionally did NOT use the variable as this way I can catch any accidental change to the query (in the variable).

where vd.vdiff_uuid = %s`, encodeString(uuid)),
},
},
{
name: "delete all",
req: &tabletmanagerdatapb.VDiffRequest{
Action: string(DeleteAction),
ActionArg: "all",
Keyspace: keyspace,
Workflow: workflow,
},
expectQueries: []string{
fmt.Sprintf(`delete from vd, vdt, vdl using _vt.vdiff as vd left join _vt.vdiff_table as vdt on (vd.id = vdt.vdiff_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is same as sqlDeleteVDiffs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I intentionally did NOT use the variable as this way I can catch any accidental change to the query (in the variable).

left join _vt.vdiff_log as vdl on (vd.id = vdl.vdiff_id)
where vd.keyspace = %s and vd.workflow = %s`, encodeString(keyspace), encodeString(workflow)),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.vde == nil {
tt.vde = vdiffenv.vde
}
for _, query := range tt.expectQueries {
vdiffenv.dbClient.ExpectRequest(query, &sqltypes.Result{}, nil)
}
got, err := tt.vde.PerformVDiffAction(ctx, tt.req)
if tt.wantErr != nil && !vterrors.Equals(err, tt.wantErr) {
t.Errorf("Engine.PerformVDiffAction() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
if tt.want != nil && !reflect.DeepEqual(got, tt.want) {
t.Errorf("Engine.PerformVDiffAction() = %v, want %v", got, tt.want)
}
})
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletmanager/vdiff/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const (
left join _vt.vdiff_log as vdl on (vd.id = vdl.vdiff_id)
where vd.keyspace = %s and vd.workflow = %s`
sqlDeleteVDiffByUUID = `delete from vd, vdt using _vt.vdiff as vd left join _vt.vdiff_table as vdt on (vd.id = vdt.vdiff_id)
and vd.vdiff_uuid = %s`
where vd.vdiff_uuid = %s`
sqlVDiffSummary = `select vd.state as vdiff_state, vd.last_error as last_error, vdt.table_name as table_name,
vd.vdiff_uuid as 'uuid', vdt.state as table_state, vdt.table_rows as table_rows,
vd.started_at as started_at, vdt.rows_compared as rows_compared, vd.completed_at as completed_at,
Expand Down