adding visibility into current mirror state #657
Conversation
|
If protos are breaking, could also remove deprecated fields |
| if err != nil { | ||
| return nil, err | ||
| } | ||
| _, err = h.ShutdownFlow(ctx, &protos.ShutdownRequest{ |
There was a problem hiding this comment.
Calling ShutdownFlow doesn't seem related to visibility. Is the purpose of this call to reduce latency to start terminating?
There was a problem hiding this comment.
this is to preserve existing behavior that waits till the workflow terminates before returning, while also ensuring we go from STATUS_TERMINATING -> STATUS_TERMINATED
| @@ -414,7 +414,7 @@ func (s *PeerFlowE2ETestSuiteSF) Test_Toast_SF(t *testing.T) { | |||
| // in a separate goroutine, wait for PeerFlowStatusQuery to finish setup | |||
| // and execute a transaction touching toast columns | |||
| go func() { | |||
| e2e.SetupCDCFlowStatusQuery(env, connectionGen) | |||
| e2e.SetupCDCFlowStateQuery(env, connectionGen) | |||
There was a problem hiding this comment.
Moving renaming into a separate PR would make this easier to review
|
PR description should explain why cdc flow SetupComplete/SnapshotComplete bools are changed into Was there a correctness issue? Was it not allowing enough info for pausing/terminating to be shown while in progress? Is there future improvements which this change is considering? |
heavycrystal
force-pushed
the
mirror-status-reporting
branch
2 times, most recently
from
e6d5827 to
24821ef
Compare
heavycrystal
force-pushed
the
mirror-status-reporting
branch
from
24821ef to
e720a67
Compare
breaks some protos and routes for clearer naming conventions, UI needs to be updated accordingly removes SetupComplete and SnapshotComplete booleans in CDCFlowState in favour of a CurrentFlowState that expresses all states of a CDC workflow in a single variable. rebase of PR: #657
breaks some protos and routes for clearer naming conventions, UI needs to be updated accordingly removes SetupComplete and SnapshotComplete booleans in CDCFlowState in favour of a CurrentFlowState that expresses all states of a CDC workflow in a single variable. rebase of PR: #657 --------- Co-authored-by: Kevin Biju <[email protected]>
breaks some protos and routes for clearer naming conventions, UI needs to be updated accordingly
removes
SetupCompleteandSnapshotCompletebooleans in CDCFlowState in favour of a CurrentFlowState that expresses all states of a CDC workflow in a single variable.