Skip to content

Augment VTAdmin test to also test VTOrc setup#283

Merged
GuptaManan100 merged 2 commits into
mainfrom
vtorc-e2e-test
Jul 12, 2022
Merged

Augment VTAdmin test to also test VTOrc setup#283
GuptaManan100 merged 2 commits into
mainfrom
vtorc-e2e-test

Conversation

@GuptaManan100

Copy link
Copy Markdown
Contributor

Description

This PR augments the VTAdmin end-to-end test to also test VTOrc. The initial cluster-setup file is changed to also launch VTOrc. After verifying that VTAdmin is working as expected, we also go on to verify that VTOrc runs as expected. This is done by stopping replication on all the valid replicas. Following this, we try and write to the primary, which only succeeds if VTOrc can fix the failure and repair replication.
An additional flag disable-active-reparent has been added to the vttablet in the test setup, just as how we recommend users to run. Furthermore, VTop only repairs replication if the source set on a tablet doesn't match the primary information. All of these pieces together mean that our test for the VTOrc setup is correct.
I have also manually verified that if VTOrc is not running, the writes are stalled indefinitely.

There is a scope for improving the test to timeout after some time. Currently, that part is left to the CI timeout configured to 1 hour in the BuildKite pipeline configuration.

Signed-off-by: Manan Gupta <manan@planetscale.com>
Signed-off-by: Manan Gupta <manan@planetscale.com>

@frouioui frouioui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we have two separate tests? One for vtadmin, another for vtorc?

@GuptaManan100

Copy link
Copy Markdown
Contributor Author

I don't think 2 are required. I actually think 1 is preferable since we should have a test that launches all the Vitess components like the users would, instead of launching only some of them in each separate test.
If I add a separate test it would pretty much be the same test without VTAdmin running and without its test. So there would just be the overhead of setting up a new cluster and nothing different.

@frouioui Is there a reason why you would prefer 2 separate tests?

@frouioui

Copy link
Copy Markdown
Member

@GuptaManan100 okay sounds good to me! I was thinking about this in case we wanted to have "more" dedicated tests for vtorc and vtadmin. But setting one big cluster with all the components is good!

@frouioui frouioui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Left two comments, otherwise, it looks good to me!

Comment thread test/endtoend/vtorc_vtadmin_test.sh
Comment thread .buildkite/pipeline.yml
@GuptaManan100 GuptaManan100 merged commit b56c353 into main Jul 12, 2022
@GuptaManan100 GuptaManan100 deleted the vtorc-e2e-test branch July 12, 2022 13:03
GuptaManan100 added a commit that referenced this pull request Jul 28, 2022
* test: augment vtadmin test to also test vtorc setup

Signed-off-by: Manan Gupta <manan@planetscale.com>

* ci: change the test name to reflect VTOrc addition too

Signed-off-by: Manan Gupta <manan@planetscale.com>
GuptaManan100 added a commit that referenced this pull request Jul 28, 2022
* test: augment vtadmin test to also test vtorc setup

Signed-off-by: Manan Gupta <manan@planetscale.com>

* ci: change the test name to reflect VTOrc addition too

Signed-off-by: Manan Gupta <manan@planetscale.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