kvserver: eagerly unquiesce ranges that use leader leases #136232

arulajmani · 2024-11-26T18:44:25Z

We eagerly unquiesce expiration based leases when starting/restarting a store. Like expiration based leases, leader leases also don't support quiescence. As such, this patch has them start off life as unquiescent.

Found while trying to port TestStoreLoadReplicaQuiescent over to use leader leases.

References #133763
Closes #136231

Release note: None

cockroach-teamcity · 2024-11-26T18:44:36Z

This change is

iskettaneh

Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani and @nvanbenschoten)

pkg/kv/kvserver/client_store_test.go line 169 at r1 (raw file):

		if leaseType == roachpb.LeaseLeader {
			// TODO(arul): add words.

Nit: Fix comment

nvanbenschoten · 2024-12-09T21:02:36Z

pkg/roachpb/data.go

@@ -2004,6 +2004,18 @@ func (l Lease) Type() LeaseType {
 	return LeaseExpiration
 }

+// SupportsQuiescence returns whether the lease supports quiescence or not.
+func (l Lease) SupportsQuiescence() bool {


Do we want to call this from shouldReplicaQuiesceRaftMuLockedReplicaMuLocked?

arulajmani

RFAL

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @iskettaneh and @nvanbenschoten)

arulajmani · 2024-12-09T22:44:20Z

pkg/roachpb/data.go

@@ -2004,6 +2004,18 @@ func (l Lease) Type() LeaseType {
 	return LeaseExpiration
 }

+// SupportsQuiescence returns whether the lease supports quiescence or not.
+func (l Lease) SupportsQuiescence() bool {


nvanbenschoten

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @iskettaneh)

-- commits line 12 at r2:
Mind adding that this fixed #135270 as well?

pkg/kv/kvserver/store.go line 2332 at r2 (raw file):

		// or expiration leases. We want to eagerly establish raft leaders for such
		// ranges and acquire leases on top of this. This happens during Raft ticks.
		// We rely on Raft pre-vote to avoid disturbance to Raft leader.s

"leader.s"

We eagerly unquiesce expiration based leases when starting/restarting a store. Like expiration based leases, leader leases also don't support quiescence. As such, this patch has them start off life as unquiescent. Found while trying to port TestStoreLoadReplicaQuiescent over to use leader leases. References cockroachdb#133763 Closes cockroachdb#136231 Closes cockroachdb#135270 Release note: None

arulajmani

Thanks!

bors r=nvanbenschoten

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @iskettaneh and @nvanbenschoten)

pkg/kv/kvserver/store.go line 2332 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

"leader.s"

Hazards of typing too fast 😅

136232: kvserver: eagerly unquiesce ranges that use leader leases r=nvanbenschoten a=arulajmani We eagerly unquiesce expiration based leases when starting/restarting a store. Like expiration based leases, leader leases also don't support quiescence. As such, this patch has them start off life as unquiescent. Found while trying to port TestStoreLoadReplicaQuiescent over to use leader leases. References #133763 Closes #136231 Release note: None 136941: rpc: avoid allocation of `TracingInternalClient` when tracing disabled r=nvanbenschoten a=nvanbenschoten Small performance improvement in the common case where tracing is disabled. ``` name old time/op new time/op delta Sysbench/SQL/1node_remote/oltp_read_only-30 6.84ms ± 2% 6.86ms ± 3% ~ (p=0.912 n=10+10) Sysbench/SQL/1node_remote/oltp_write_only-30 4.16ms ± 2% 4.17ms ± 2% ~ (p=0.529 n=10+10) Sysbench/SQL/1node_remote/oltp_read_write-30 11.2ms ± 2% 11.2ms ± 2% ~ (p=0.912 n=10+10) Sysbench/SQL/1node_remote/oltp_point_select-30 458µs ± 2% 459µs ± 3% ~ (p=0.971 n=10+10) Sysbench/SQL/3node/oltp_read_only-30 7.02ms ± 2% 7.06ms ± 2% ~ (p=0.529 n=10+10) Sysbench/SQL/3node/oltp_write_only-30 4.42ms ± 2% 4.41ms ± 2% ~ (p=0.604 n=10+9) Sysbench/SQL/3node/oltp_read_write-30 11.7ms ± 2% 11.7ms ± 2% ~ (p=0.684 n=10+10) Sysbench/SQL/3node/oltp_point_select-30 473µs ± 2% 474µs ± 2% ~ (p=0.796 n=10+10) Sysbench/KV/1node_remote/oltp_read_only-30 2.67ms ± 2% 2.68ms ± 2% ~ (p=0.243 n=9+10) Sysbench/KV/1node_remote/oltp_write_only-30 2.20ms ± 3% 2.19ms ± 2% ~ (p=0.739 n=10+10) Sysbench/KV/1node_remote/oltp_read_write-30 5.04ms ± 2% 5.05ms ± 2% ~ (p=0.853 n=10+10) Sysbench/KV/1node_remote/oltp_point_select-30 125µs ± 2% 125µs ± 1% ~ (p=0.278 n=10+9) Sysbench/KV/3node/oltp_read_only-30 2.79ms ± 2% 2.79ms ± 2% ~ (p=0.579 n=10+10) Sysbench/KV/3node/oltp_write_only-30 2.51ms ± 2% 2.50ms ± 1% ~ (p=0.762 n=10+8) Sysbench/KV/3node/oltp_read_write-30 5.57ms ± 2% 5.55ms ± 1% ~ (p=0.968 n=10+9) Sysbench/KV/3node/oltp_point_select-30 131µs ± 4% 132µs ± 3% ~ (p=0.481 n=10+10) name old alloc/op new alloc/op delta Sysbench/SQL/1node_remote/oltp_read_only-30 1.35MB ± 0% 1.35MB ± 0% ~ (p=0.063 n=10+10) Sysbench/SQL/1node_remote/oltp_write_only-30 670kB ± 0% 670kB ± 0% ~ (p=0.739 n=10+10) Sysbench/SQL/1node_remote/oltp_read_write-30 2.11MB ± 0% 2.11MB ± 0% ~ (p=0.063 n=10+10) Sysbench/SQL/1node_remote/oltp_point_select-30 42.2kB ± 0% 42.2kB ± 0% ~ (p=0.540 n=10+10) Sysbench/SQL/3node/oltp_read_only-30 1.42MB ± 4% 1.40MB ± 1% ~ (p=0.965 n=10+8) Sysbench/SQL/3node/oltp_write_only-30 1.15MB ± 3% 1.13MB ± 8% ~ (p=0.165 n=10+10) Sysbench/SQL/3node/oltp_read_write-30 2.77MB ± 1% 2.76MB ± 1% ~ (p=0.190 n=10+10) Sysbench/SQL/3node/oltp_point_select-30 44.9kB ± 1% 47.2kB ±12% ~ (p=0.101 n=8+10) Sysbench/KV/1node_remote/oltp_read_only-30 791kB ± 0% 790kB ± 0% ~ (p=0.280 n=10+10) Sysbench/KV/1node_remote/oltp_write_only-30 334kB ± 2% 334kB ± 2% ~ (p=0.971 n=10+10) Sysbench/KV/1node_remote/oltp_read_write-30 1.12MB ± 0% 1.12MB ± 0% ~ (p=0.447 n=10+9) Sysbench/KV/1node_remote/oltp_point_select-30 16.4kB ± 0% 16.4kB ± 0% ~ (p=0.956 n=10+10) Sysbench/KV/3node/oltp_read_only-30 808kB ± 1% 809kB ± 0% ~ (p=0.211 n=10+9) Sysbench/KV/3node/oltp_write_only-30 618kB ± 1% 621kB ± 1% ~ (p=0.075 n=10+10) Sysbench/KV/3node/oltp_read_write-30 1.45MB ± 1% 1.45MB ± 0% ~ (p=0.315 n=10+8) Sysbench/KV/3node/oltp_point_select-30 17.1kB ± 1% 17.0kB ± 1% ~ (p=0.382 n=10+10) name old allocs/op new allocs/op delta Sysbench/KV/1node_remote/oltp_point_select-30 211 ± 0% 210 ± 0% -0.47% (p=0.000 n=10+10) Sysbench/KV/3node/oltp_point_select-30 213 ± 0% 212 ± 0% -0.47% (p=0.000 n=10+10) Sysbench/SQL/3node/oltp_read_write-30 17.5k ± 1% 17.4k ± 1% -0.46% (p=0.029 n=10+10) Sysbench/KV/1node_remote/oltp_read_only-30 3.99k ± 0% 3.97k ± 0% -0.35% (p=0.000 n=10+10) Sysbench/KV/3node/oltp_read_only-30 4.02k ± 0% 4.01k ± 0% -0.34% (p=0.000 n=10+10) Sysbench/KV/1node_remote/oltp_write_only-30 3.15k ± 0% 3.14k ± 0% -0.32% (p=0.000 n=10+10) Sysbench/KV/3node/oltp_read_write-30 8.63k ± 0% 8.61k ± 0% -0.31% (p=0.000 n=10+9) Sysbench/KV/1node_remote/oltp_read_write-30 7.20k ± 0% 7.18k ± 0% -0.29% (p=0.000 n=10+10) Sysbench/SQL/1node_remote/oltp_read_only-30 7.70k ± 0% 7.67k ± 0% -0.27% (p=0.000 n=10+10) Sysbench/SQL/3node/oltp_point_select-30 459 ± 0% 458 ± 0% -0.27% (p=0.013 n=9+9) Sysbench/SQL/1node_remote/oltp_point_select-30 440 ± 0% 439 ± 0% -0.23% (p=0.000 n=10+9) Sysbench/SQL/1node_remote/oltp_write_only-30 6.97k ± 0% 6.95k ± 0% -0.20% (p=0.000 n=10+9) Sysbench/KV/3node/oltp_write_only-30 4.41k ± 0% 4.40k ± 0% -0.18% (p=0.000 n=9+10) Sysbench/SQL/1node_remote/oltp_read_write-30 14.7k ± 0% 14.7k ± 0% -0.16% (p=0.002 n=10+9) Sysbench/SQL/3node/oltp_read_only-30 7.96k ± 1% 7.98k ± 1% ~ (p=0.618 n=9+10) Sysbench/SQL/3node/oltp_write_only-30 9.32k ± 1% 9.24k ± 2% ~ (p=0.211 n=9+10) ``` Epic: None Release note: None Co-authored-by: Arul Ajmani <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]>

craig · 2024-12-10T00:19:29Z

Build failed (retrying...):

macos_amd64_build

136232: kvserver: eagerly unquiesce ranges that use leader leases r=nvanbenschoten a=arulajmani We eagerly unquiesce expiration based leases when starting/restarting a store. Like expiration based leases, leader leases also don't support quiescence. As such, this patch has them start off life as unquiescent. Found while trying to port TestStoreLoadReplicaQuiescent over to use leader leases. References #133763 Closes #136231 Release note: None Co-authored-by: Arul Ajmani <[email protected]>

craig · 2024-12-10T00:33:32Z

Build failed:

unit_tests

arulajmani · 2024-12-10T14:44:40Z

bors retry

craig · 2024-12-10T15:24:39Z

Build succeeded:

arulajmani requested review from nvanbenschoten and iskettaneh November 26, 2024 18:44

arulajmani requested a review from a team as a code owner November 26, 2024 18:44

iskettaneh reviewed Nov 28, 2024

View reviewed changes

nvanbenschoten mentioned this pull request Dec 9, 2024

roachtest: replicate/wide failed #135270

Closed

nvanbenschoten reviewed Dec 9, 2024

View reviewed changes

arulajmani force-pushed the ll-TestStoreLoadReplicaQuiescent branch from 1aea6d4 to 4e40d3c Compare December 9, 2024 22:43

arulajmani commented Dec 9, 2024

View reviewed changes

arulajmani requested review from iskettaneh and nvanbenschoten December 9, 2024 22:44

nvanbenschoten approved these changes Dec 9, 2024

View reviewed changes

arulajmani force-pushed the ll-TestStoreLoadReplicaQuiescent branch from 4e40d3c to bc23142 Compare December 10, 2024 00:06

arulajmani commented Dec 10, 2024

View reviewed changes

craig bot merged commit 28d9b53 into cockroachdb:master Dec 10, 2024
21 of 23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kvserver: eagerly unquiesce ranges that use leader leases #136232

kvserver: eagerly unquiesce ranges that use leader leases #136232

arulajmani commented Nov 26, 2024 •

edited

Loading

cockroach-teamcity commented Nov 26, 2024

iskettaneh left a comment

nvanbenschoten Dec 9, 2024

arulajmani Dec 9, 2024

arulajmani left a comment

arulajmani Dec 9, 2024

nvanbenschoten left a comment

arulajmani left a comment

craig bot commented Dec 10, 2024

craig bot commented Dec 10, 2024

arulajmani commented Dec 10, 2024

craig bot commented Dec 10, 2024

kvserver: eagerly unquiesce ranges that use leader leases #136232

kvserver: eagerly unquiesce ranges that use leader leases #136232

Conversation

arulajmani commented Nov 26, 2024 • edited Loading

cockroach-teamcity commented Nov 26, 2024

iskettaneh left a comment

Choose a reason for hiding this comment

nvanbenschoten Dec 9, 2024

Choose a reason for hiding this comment

arulajmani Dec 9, 2024

Choose a reason for hiding this comment

arulajmani left a comment

Choose a reason for hiding this comment

arulajmani Dec 9, 2024

Choose a reason for hiding this comment

nvanbenschoten left a comment

Choose a reason for hiding this comment

arulajmani left a comment

Choose a reason for hiding this comment

craig bot commented Dec 10, 2024

craig bot commented Dec 10, 2024

arulajmani commented Dec 10, 2024

craig bot commented Dec 10, 2024

arulajmani commented Nov 26, 2024 •

edited

Loading