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

fix: Infinite logs in case of non-existent stream logs #17004

Merged
merged 2 commits into from
Oct 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 2 additions & 2 deletions go/vt/vtctl/workflow/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,8 @@ ORDER BY

if stream.Id > streamLog.StreamId {
s.Logger().Warningf("Found stream log for nonexistent stream: %+v", streamLog)
// This can happen on manual/failed workflow cleanup so keep going.
continue
// This can happen on manual/failed workflow cleanup so move to the next log.
break
}

// stream.Id == streamLog.StreamId
Expand Down
57 changes: 57 additions & 0 deletions go/vt/vtctl/workflow/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1811,3 +1811,60 @@ func createReadVReplicationWorkflowFunc(t *testing.T, workflowType binlogdatapb.
}, nil
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a brief comment about what this is testing as it's not obvious from the name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing out that @mattlord! I've changed the test func name and added the comment.

func TestGetWorkflowsSingleStream(t *testing.T) {
ctx := context.Background()

sourceKeyspace := "source_keyspace"
targetKeyspace := "target_keyspace"
workflow := "test_workflow"

sourceShards := []string{"-"}
targetShards := []string{"-"}

te := newTestMaterializerEnv(t, ctx, &vtctldatapb.MaterializeSettings{
SourceKeyspace: sourceKeyspace,
TargetKeyspace: targetKeyspace,
Workflow: workflow,
TableSettings: []*vtctldatapb.TableMaterializeSettings{
{
TargetTable: "table1",
SourceExpression: fmt.Sprintf("select * from %s", "table1"),
},
{
TargetTable: "table2",
SourceExpression: fmt.Sprintf("select * from %s", "table2"),
},
},
}, sourceShards, targetShards)

logResult := sqltypes.MakeTestResult(
sqltypes.MakeTestFields("id|vrepl_id|type|state|message|created_at|updated_at|`count`", "int64|int64|varchar|varchar|varchar|varchar|varchar|int64"),
"1|0|State Change|Running|test message for non-existent 1|2006-01-02 15:04:05|2006-01-02 15:04:05|1",
"2|0|State Change|Stopped|test message for non-existent 2|2006-01-02 15:04:06|2006-01-02 15:04:06|1",
"3|1|State Change|Running|log message|2006-01-02 15:04:07|2006-01-02 15:04:07|1",
)

te.tmc.expectVRQuery(200, "select vrepl_id, table_name, lastpk from _vt.copy_state where vrepl_id in (1) and id in (select max(id) from _vt.copy_state where vrepl_id in (1) group by vrepl_id, table_name)", &sqltypes.Result{})
te.tmc.expectVRQuery(200, "select id from _vt.vreplication where db_name = 'vt_target_keyspace' and workflow = 'test_workflow'", &sqltypes.Result{})
te.tmc.expectVRQuery(200, "select id, vrepl_id, type, state, message, created_at, updated_at, `count` from _vt.vreplication_log where vrepl_id in (1) order by vrepl_id asc, id asc", logResult)

res, err := te.ws.GetWorkflows(ctx, &vtctldatapb.GetWorkflowsRequest{
Keyspace: targetKeyspace,
Workflow: workflow,
IncludeLogs: true,
})
require.NoError(t, err)

assert.Len(t, res.Workflows, 1)
assert.NotNil(t, res.Workflows[0].ShardStreams["-/cell-0000000200"])
assert.Len(t, res.Workflows[0].ShardStreams["-/cell-0000000200"].Streams, 1)

gotLogs := res.Workflows[0].ShardStreams["-/cell-0000000200"].Streams[0].Logs

// The non-existent stream logs shouldn't be part of the result
assert.Len(t, gotLogs, 1)
assert.Equal(t, gotLogs[0].Message, "log message")
assert.Equal(t, gotLogs[0].State, "Running")
assert.Equal(t, gotLogs[0].Id, int64(3))
}
Loading