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

vreplication: vdiff #5367

Merged
merged 28 commits into from
Nov 3, 2019
Merged

vreplication: vdiff #5367

merged 28 commits into from
Nov 3, 2019

Conversation

sougou
Copy link
Contributor

@sougou sougou commented Oct 28, 2019

This change implements vdiff, which can compare the source and target tables of a workflow.
This works for only for deterministic streams. Non-deterministic streams are likely to report differences.

Doesn't look like this change can be broken out into smaller pieces.

Copy link
Member

@deepthi deepthi left a comment

Choose a reason for hiding this comment

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

LGTM. A few questions inline.

if err != nil {
return fmt.Errorf("WaitMasterPos: MasterPosition failed: %v", err)
}
if mpos.AtLeast(targetPos) {
Copy link
Member

Choose a reason for hiding this comment

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

If this condition is false, we fall through to WaitUntilPositionCommand. Is that safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should not happen for master because it will always be up-to-date. Even if it does, the wait command will just return an error, which is fine.

_, statErr := os.Stat("/tmp/vtSimulateFetchFailures")
simulateFailures = statErr == nil
}
// TODO(sougou): this file should be renamed.
Copy link
Member

Choose a reason for hiding this comment

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

Now's a good time :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I plan to address these as part of the V2 and SBR cleanups.

@@ -317,6 +317,9 @@ var commands = []commandGroup{
{"VerticalSplitClone", commandVerticalSplitClone,
"<from_keyspace> <to_keyspace> <tables>",
"Start the VerticalSplitClone process to perform vertical resharding. Example: SplitClone from_ks to_ks 'a,/b.*/'"},
{"VDiff", commandVDiff,
"-workflow=<workflow> <target keyspace> [-source_cell=<cell>] [-target_cell=<cell>] [-tablet_types=REPLICA] [-filtered_replication_wait_time=30s]",
Copy link
Member

Choose a reason for hiding this comment

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

Does order matter? Does <target keyspace> need to be at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Order doesn't matter. The ordering of the help line implies that workflow is a mandatory flag and target_keyspace is a mandatory argument. The rest are optional.

source: sqltypes.MakeTestStreamingResults(fields,
"1|3",
"2|4",
"---",
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean?

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
It now waits indefinitely until at least one tablet is available.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
It now picks equally from all tablet types.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
* WaitForPos previously worked only for replicas. It's been
  updated to work correctly for masters also.

* resultReader keyspace was hardcoded to source. The keyspace
  name is now passed as input param.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Some tests still depend on the healthcheck timeout options.
So, tablet picker still needs to accept them as arguments.

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
@sougou sougou force-pushed the ss-vrepl-vdiff branch 2 times, most recently from e9ad938 to 09987c4 Compare November 3, 2019 01:06
change test to handle new mastership rules

Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Signed-off-by: Sugu Sougoumarane <ssougou@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants