kvserver, kvclient: allow Rangefeeds to run over non-voting replicas #59507
Conversation
|
This change is |
aayushshah15
force-pushed
the
rangefeeds-on-non-voters
branch
from
864ee08 to
64ba572
Compare
There was a problem hiding this comment.
Reviewed 1 of 2 files at r4.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15)
pkg/kv/kvserver/replica_rangefeed_test.go, line 121 at r4 (raw file):
tc.SplitRangeOrFatal(t, startKey) if withNonVoters {
Instead of the RunTrueAndFalse, maybe we just add 2 voters and 2 non-voters and make sure that all 5 replicas can serve rangefeeds?
pkg/kv/kvserver/replica_rangefeed_test.go, line 161 at r4 (raw file):
WithDiff: true, } pErr := store.RangeFeed(&req, stream)
Is this test testing the change you made in this PR? It's a good test case to add regardless, but it doesn't seem to be going through the DistSender. Does the test pass even if you revert the other half of this change?
|
woops, I didn't realize this wasn't a draft PR. Was lazily prototyping & wanted CI to run on it. |
|
@amruss this is the PR you'll want to wait on before testing CDC + multi-region. |
aayushshah15
force-pushed
the
rangefeeds-on-non-voters
branch
from
64ba572 to
bbe89cf
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/replica_rangefeed_test.go, line 121 at r4 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Instead of the RunTrueAndFalse, maybe we just add 2 voters and 2 non-voters and make sure that all 5 replicas can serve rangefeeds?
Done.
pkg/kv/kvserver/replica_rangefeed_test.go, line 161 at r4 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Is this test testing the change you made in this PR? It's a good test case to add regardless, but it doesn't seem to be going through the DistSender. Does the test pass even if you revert the other half of this change?
Added a new test.
aayushshah15
force-pushed
the
rangefeeds-on-non-voters
branch
from
bbe89cf to
12389de
Compare
There was a problem hiding this comment.
=== RUN TestRangefeedIsRoutedToNonVoter
test_log_scope.go:73: test logs captured to: /go/src/github.com/cockroachdb/cockroach/artifacts/logTestRangefeedIsRoutedToNonVoter106363519
test_log_scope.go:74: use -show-logs to present logs inline
panic: test timed out after 15m0s
goroutine 53 [chan receive, 14 minutes]:
github.com/cockroachdb/cockroach/pkg/kv/kvserver_test.TestRangefeedIsRoutedToNonVoter(0xc000103500)
/go/src/github.com/cockroachdb/cockroach/pkg/kv/kvserver/client_rangefeed_test.go:231 +0x82e
testing.tRunner(0xc000103500, 0x4e19a48)
/usr/local/go/src/testing/testing.go:1123 +0xef
created by testing.(*T).Run
/usr/local/go/src/testing/testing.go:1168 +0x2b3
Reviewed 19 of 19 files at r5.
Reviewable status:
Prior to this commit, RangeFeeds only worked over voting replicas. This included followers as well as the leaseholder. This commit allows rangefeeds to run over non-voting replicas as well. Release justification: low risk high reward change to be able to fuel CDC streams using non-voting replicas Release note: None
aayushshah15
force-pushed
the
rangefeeds-on-non-voters
branch
from
12389de to
839de54
Compare
|
Fixed CI. TFTR! bors r+ |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
bors r- |
|
Canceled. |
|
bors r+ |
|
Build succeeded: |
Fixes #59454.
Release justification: low risk high benefit change to be able to fuel CDC streams using non-voting replicas
Release note: None