kvserver/closedts: add TestPolicyRefresherWithLatencies#144482
kvserver/closedts: add TestPolicyRefresherWithLatencies#144482craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
|
I recall you mentioned we prefer testing as much as possible sync rather than async. So I'm manually invoking the methods and verifying the expected outcomes here. Alternatively, we could call |
ccd2b08 to
d209694
Compare
d209694 to
3688eb4
Compare
wenyihu6
left a comment
There was a problem hiding this comment.
Do we consider #143888 resolved with this PR? I recall there was some interest in adding roachtests with more distant regions. I've enabled auto-tuning metamorphically for all existing follower reads roachtests, so auto-tuning will be tested regardless of region distance or latency. So far, there haven't been any real test failures. If we want to extend test coverage, we can add tests involving more distant regions but it may introduce some flakiness. Wdyt?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @arulajmani)
arulajmani
left a comment
There was a problem hiding this comment.
Let's chat about whether we want to add any more roachtests before closing the linked issue offline.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/kv/kvserver/closedts/policyrefresher/policy_refresher_test.go line 292 at r2 (raw file):
// Enable global reads and set furthest node to 1. We should get // LEAD_FOR_GLOBAL_READS_WITH_NO_LATENCY_INFO policy before cache update.
nit: "before the cache update"
arulajmani
left a comment
There was a problem hiding this comment.
Actually, not much to chat -- I just checked, and the current follower reads roachtests uses us-east, us-west, and eu-west as the regions. That should place replicas in different buckets, so we should have good coverage with the metamorphic changes. Let's close #143888 out.
Great work here!
Reviewable status:
3688eb4 to
e246347
Compare
wenyihu6
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/closedts/policyrefresher/policy_refresher_test.go line 292 at r2 (raw file):
Previously, arulajmani (Arul Ajmani) wrote…
nit: "before the cache update"
Done.
e246347 to
1c7efbc
Compare
|
Last push was to rebase on master to include a flaky test fix for TestAuthenticationAndHBARules. |
This commit adds a new test, TestPolicyRefresherWithLatencies, which verifies that the policy refresher correctly updates latency information using mocked latency map data. Resolves: cockroachdb#143888 Release note: none
1c7efbc to
5dbc33c
Compare
|
Last push was to rebase on master to include a flaky test fix for TestLeaderlessWatcherErrorRefreshedOnUnavailabilityTransition. |
|
TFTR! bors r=arulajmani |
|
Build failed: |
|
bors retry |
|
Build succeeded: |
This commit adds a new test, TestPolicyRefresherWithLatencies, which verifies
that the policy refresher correctly updates latency information using mocked
latency map data.
Resolves: #143888
Release note: none