-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
VSchema Validation on ReshardWorkflow Creation #7977
Conversation
Signed-off-by: Malcolm Akinje <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Malcolm!
This is looking good. One thing that I was discussing with @rohit-nayak-ps is that we should make sure to ignore ghost schema changes from these validations.
However, there are a few other places we need to update before fully adopting those conventions (e.g VDiff should ignore tables that are related to ghost).
We can work on that as separate PR.
Does this mean that in the presence of gh-ost artifact tables workflow creation will always fail (with the changes in this PR)? |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. One comment that I think can be done as a follow-up change.
@@ -199,7 +217,7 @@ func (wr *Wrangler) ValidateSchemaKeyspace(ctx context.Context, keyspace string, | |||
} | |||
sort.Strings(shards) | |||
if len(shards) == 1 { | |||
return wr.ValidateSchemaShard(ctx, keyspace, shards[0], excludeTables, includeViews) | |||
return wr.ValidateSchemaShard(ctx, keyspace, shards[0], excludeTables, includeViews, false /*includeVSchema*/) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow-up, I think it would be great to add the VSchema validation to ValidateSchemaKeyspace
as well
Signed-off-by: Malcolm Akinje [email protected]
Description
This PR adds in vschema validation during creation of a ReshardWorkflow. If any of the source shards have tables in their
_vt
database that are not in the vschema then the creation of the ReshardWorkflow will fail. We will then return which shards failed their vschema checks and what tables caused their respective failures.Related Issue(s)
#7620
Checklist
Deployment Notes