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

Online DDL: progress & ETA for Vreplication migrations #8015

Merged
merged 5 commits into from
Jun 15, 2021

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented May 2, 2021

Description

#7980 introduced VReplicaiton tracking for rows_copied. This PR both intensifies that tracking, and follows up in Online DDL: the online DDL executor periodically checks rows_copied from _vt.vstream, compares with estimated table_rows, looks at started_timestamp, and generates values for progress and for eta_seconds.

Online DDL does so every 1 minute, normally, which means the valuee progress and eta_seconds are estimates only. Plus, vcopier itself only updates rows_copied every 30s, so we could get almost a 90s stale value for progress and eta_seconds. Which is acceptable to us at this tim.

Related Issue(s)

Checklist

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

Deployment Notes

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Contributor Author

This PR is generally good to go. I'm struggling if/whether to add endtoend test that checks this specific behavior.

Obviously it would be nice to test the behavior. But then, I'm not sure how to go about it. It would take a relatively long migration to be able to test this. We'd need a migration that is absolutely known to run for over 90sec, and that we can ensure will show a non-zero, non-100% progress value, at some point. This can be done, at a cost. It will take long minutes to populate and test such a migrated table, and I'm concerned about impact to CI.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach
Copy link
Contributor Author

shlomi-noach commented Jun 7, 2021

Now also complements #8255 and updates rows_copied for VReplication migrations. Note that while migration is running rows_copied is only updated once per minute, and based on vreplication's rows_copied which in itself can be delayed.

When the migration completes, rows_copied is ensured to be the final & exact value.

@shlomi-noach shlomi-noach added Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Jun 7, 2021
@shlomi-noach shlomi-noach marked this pull request as ready for review June 7, 2021 10:13
@shlomi-noach
Copy link
Contributor Author

This is now ready for review.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Query Serving Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants