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

[workflow] Add vreplication_log data to workflow protos, and VtctldServer.GetWorkflows method #8261

Merged
merged 6 commits into from
Jun 13, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Jun 4, 2021

Description

This enhances the GetWorkflows endpoint in VtcltdServer to also, on a best-effort basis, query the _vt.vreplication_log table for all streams involved in each workflow. On tablets that are running with older versions of vitess (which do not have the log table), then the log fetch queries should fail, and we will set LogFetchError on each stream in each workflow in the response, but the overall RPC will still be non-error.

Here is this change running, accessed via vtadmin, on the 303_reshard local example:

Screen Shot 2021-06-03 at 6 33 22 PM

Related Issue(s)

Closes #8260

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

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

This looks really really great to me. I messed with it a bit locally and the data model is purrfect for what I need. 😈

The vexec/query planner parts seem straightforward to me, but you might want another set of eyes on the particularly Vitess-y parts. Up to you!

Thank youuuu!!!

// each _vt.vreplication row, we also sorted each ShardStreams
// slice by ascending id, and our _vt.vreplication_log query
// ordered by (stream_id ASC, id ASC), so we can walk the
// streams in index order in O(n) amortized over all the rows
Copy link
Contributor

Choose a reason for hiding this comment

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

🤓

Copy link
Contributor

@rohit-nayak-ps rohit-nayak-ps left a comment

Choose a reason for hiding this comment

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

lgtm

ajm188 added 6 commits June 13, 2021 14:46
The intention here is that, for `vreplication_log` queries, we will want
to have different `stream_id` bindvars for each target tablet in the
query. Therefore in the next set of changes, we'll provided a second
implementation of `QueryPlan` that allows a different query (with
different bind vars) to be run on each target.

Signed-off-by: Andrew Mason <[email protected]>
@ajm188 ajm188 force-pushed the am_vreplication_log branch from 4131b2d to 24b84a4 Compare June 13, 2021 18:48
@ajm188 ajm188 merged commit f97942a into vitessio:main Jun 13, 2021
@ajm188 ajm188 deleted the am_vreplication_log branch June 13, 2021 19:41
ajm188 added a commit to tinyspeck/vitess that referenced this pull request Jun 15, 2021
[workflow] Add vreplication_log data to workflow protos, and `VtctldServer.GetWorkflows` method

Signed-off-by: Andrew Mason <[email protected]>
ajm188 added a commit to tinyspeck/vitess that referenced this pull request Jul 23, 2021
[workflow] Add vreplication_log data to workflow protos, and `VtctldServer.GetWorkflows` method

Signed-off-by: Andrew Mason <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vtctld] Add vreplication_log data to VtctldServer.GetWorkflows
3 participants