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

kvserver, roachtest: add non-voting replicas to follower_reads roachtest #59678

Closed
aayushshah15 opened this issue Feb 1, 2021 · 0 comments · Fixed by #61836
Closed

kvserver, roachtest: add non-voting replicas to follower_reads roachtest #59678

aayushshah15 opened this issue Feb 1, 2021 · 0 comments · Fixed by #61836
Assignees
Labels
A-kv-replication Relating to Raft, consensus, and coordination. O-roachtest

Comments

@aayushshah15
Copy link
Contributor

The follower reads roachtest creates a geo-distributed roachprod cluster and asserts that AS OF SYSTEM TIME queries are served by follower replicas on sufficiently old data. Currently, this test only uses voting replicas. We should add a variant of this test that exercises the same workload over a table that has non-voting replicas, and ensures that those replicas can serve follower reads.

@aayushshah15 aayushshah15 added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Feb 1, 2021
@aayushshah15 aayushshah15 self-assigned this Feb 1, 2021
@aayushshah15 aayushshah15 added A-kv-replication Relating to Raft, consensus, and coordination. O-roachtest and removed C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) labels Feb 1, 2021
aayushshah15 added a commit to aayushshah15/cockroach that referenced this issue Feb 5, 2021
This commit adds two new variants to the follower reads roachtest: one
that sets up a cluster with 3 voting replicas and 2 non-voting replicas,
and another that has 1 voting replica and 2 non-voting replicas.

Resolves cockroachdb#59678

Release note: None
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Mar 11, 2021
… tables

Fixes cockroachdb#59678.
Requires cockroachdb#59839.

This commit expands the existing `follower-reads` roachtest into four variants:
- follower-reads/survival=region/locality=global
- follower-reads/survival=region/locality=regional
- follower-reads/survival=zone/locality=global
- follower-reads/survival=zone/locality=regional

As the names indicate, these four variants are adapted to use the [new
multi-region abstractions](https://www.cockroachlabs.com/docs/v21.1/multiregion-overview.html).
Specifically, two of the variants configure the test database with a
REGION survival goal while two of the variants configure the test
database with a ZONE survival goal. The ZONE variant give us testing
coverage for serving follower reads off of non-voting replicas, because
only the primary region of the database will house voting replicas.

The use of multi-region abstractions also extends to table locality
settings. Two of the variants configure the table with a REGIONAL
locality setting. This necessitates the use of stale reads to read from
followers, as the test had been doing before. However, the other two
variants configure the table with a GLOBAL locality setting. Because
GLOBAL tables can serve non-stale reads from followers, the test can
omit the `AS OF SYSTEM TIME follower_read_timestamp()` clause in these
variants and test that GLOBAL tables provide low-latency consistent
reads from all regions.

I remembered a little late that this conflicts with cockroachdb#59843, which adds
support for non-voting replicas to this test in a bit of a different
way. I'm curious how people feel about these two approaches. I slightly
prefer the use of high-level multi-region abstractions instead of
lower-level zone configs, because that's what we're going to push people
towards going forward and it expands our testing coverage. But if we go
with that approach, we should probably address the TODO left here about
more carefully checking voting vs. non-voting replica placement.
craig bot pushed a commit that referenced this issue Mar 14, 2021
61836: roachtest/follower-reads: adopt multi-region abstractions, add GLOBAL tables r=nvanbenschoten a=nvanbenschoten

Fixes #59678.
Requires #59839.

This commit expands the existing `follower-reads` roachtest into four variants:
- `follower-reads/survival=region/locality=global`
- `follower-reads/survival=region/locality=regional`
- `follower-reads/survival=zone/locality=global`
- `follower-reads/survival=zone/locality=regional`

As the names indicate, these four variants are adapted to use the [new
multi-region abstractions](https://www.cockroachlabs.com/docs/v21.1/multiregion-overview.html).
Specifically, two of the variants configure the test database with a
REGION survival goal while two of the variants configure the test
database with a ZONE survival goal. The ZONE variant give us testing
coverage for serving follower reads off of non-voting replicas, because
only the primary region of the database will house voting replicas.

The use of multi-region abstractions also extends to table locality
settings. Two of the variants configure the table with a REGIONAL
locality setting. This necessitates the use of stale reads to read from
followers, as the test had been doing before. However, the other two
variants configure the table with a GLOBAL locality setting. Because
GLOBAL tables can serve non-stale reads from followers, the test can
omit the `AS OF SYSTEM TIME follower_read_timestamp()` clause in these
variants and test that GLOBAL tables provide low-latency consistent
reads from all regions.

I remembered a little late that this conflicts with #59843, which adds
support for non-voting replicas to this test in a bit of a different
way. I'm curious how people feel about these two approaches. I slightly
prefer the use of high-level multi-region abstractions instead of
lower-level zone configs, because that's what we're going to push people
towards going forward and it expands our testing coverage. But if we go
with that approach, we should probably address the TODO left here about
more carefully checking voting vs. non-voting replica placement.

Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in f51f0bc Mar 14, 2021
nvanbenschoten added a commit to nvanbenschoten/cockroach that referenced this issue Apr 1, 2021
… tables

Fixes cockroachdb#59678.

This commit expands the existing `follower-reads` roachtest into four variants:
- follower-reads/survival=region/locality=global
- follower-reads/survival=region/locality=regional
- follower-reads/survival=zone/locality=global
- follower-reads/survival=zone/locality=regional

As the names indicate, these four variants are adapted to use the [new
multi-region abstractions](https://www.cockroachlabs.com/docs/v21.1/multiregion-overview.html).
Specifically, two of the variants configure the test database with a
REGION survival goal while two of the variants configure the test
database with a ZONE survival goal. The ZONE variant give us testing
coverage for serving follower reads off of non-voting replicas, because
only the primary region of the database will house voting replicas.

The use of multi-region abstractions also extends to table locality
settings. Two of the variants configure the table with a REGIONAL
locality setting. This necessitates the use of stale reads to read from
followers, as the test had been doing before. However, the other two
variants configure the table with a GLOBAL locality setting. Because
GLOBAL tables can serve non-stale reads from followers, the test can
omit the `AS OF SYSTEM TIME follower_read_timestamp()` clause in these
variants and test that GLOBAL tables provide low-latency consistent
reads from all regions.

Release note: None
Release justification: testing only
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-replication Relating to Raft, consensus, and coordination. O-roachtest
Projects
None yet
1 participant