Merge #144485

144485: kvserver: shorten leader-lease min-expiration grace period r=arulajmani a=miraradeva

Previously, when constructing a new leader lease, the min-expiration of the new lease was set to the maximum of the previous lease expiration and now + 6s (where the 6s corresponded to the lease duration). While the former is needed for correctness (to ensure the lease disjointness property), the latter is just a grace period for the new leader to finish fortifying. When now + 6s was introduced, the duration of store liveness support was also 6s, so this matched the duration of a typical leader lease. However, since then the store liveness support duration was decreased to 3s, and now the 6s seems excessive for a grace period.

This became evident from the last remaining case
(failover/non-system/blackhole-recv) where the failover duration of expiration leases was a little better than leader leases (see #133612). In this scenario, the old leader was not able to receive messages but was successfully proposing new leader leases with min-expirations of 6s into the future. When a new leader was eventually elected, it had to wait out that min-expiration before starting a new valid lease.

This commit introduces a new config setting for the fortification grace period and sets it to a default of 3s.

Informs: #133612

Release note: None

Co-authored-by: Mira Radeva <[email protected]>

Author: craig[bot]

pkg/base/config.go

@@ -235,6 +235,11 @@ var (
 	defaultStoreLivenessSupportDuration = envutil.EnvOrDefaultDuration(
 		"COCKROACH_STORE_LIVENESS_SUPPORT_DURATION", 3*time.Second)
 
+	// defaultFortificationGracePeriod is the default value for
+	// FortificationGracePeriod.
+	defaultFortificationGracePeriod = envutil.EnvOrDefaultDuration(
+		"COCKROACH_RAFT_FORTIFICATION_GRACE_PERIOD", 3*time.Second)
+
 	// defaultRaftTickInterval is the default resolution of the Raft timer.
 	defaultRaftTickInterval = envutil.EnvOrDefaultDuration(
 		"COCKROACH_RAFT_TICK_INTERVAL", 500*time.Millisecond)
@@ -590,6 +595,10 @@ type RaftConfig struct {
 	// stores request and extend.
 	StoreLivenessSupportDuration time.Duration
 
+	// FortificationGracePeriod is the minimum validity of a new leader lease to
+	// allow for the new leader to fortify.
+	FortificationGracePeriod time.Duration
+
 	// RangeLeaseRaftElectionTimeoutMultiplier specifies the range lease duration.
 	RangeLeaseDuration time.Duration
 	// RangeLeaseRenewalFraction specifies what fraction the range lease renewal
@@ -706,6 +715,9 @@ func (cfg *RaftConfig) SetDefaults() {
 	if cfg.StoreLivenessSupportDuration == 0 {
 		cfg.StoreLivenessSupportDuration = defaultStoreLivenessSupportDuration
 	}
+	if cfg.FortificationGracePeriod == 0 {
+		cfg.FortificationGracePeriod = defaultFortificationGracePeriod
+	}
 	if cfg.RangeLeaseDuration == 0 {
 		cfg.RangeLeaseDuration = defaultRangeLeaseDuration
 	}

pkg/base/testdata/raft_config

@@ -9,6 +9,7 @@ echo
  RaftHeartbeatIntervalTicks: (int64) 2,
  StoreLivenessHeartbeatInterval: (time.Duration) 1s,
  StoreLivenessSupportDuration: (time.Duration) 3s,
+ FortificationGracePeriod: (time.Duration) 3s,
  RangeLeaseDuration: (time.Duration) 6s,
  RangeLeaseRenewalFraction: (float64) 0.5,
  RaftEnableCheckQuorum: (bool) true,

pkg/kv/kvserver/leases/build.go

@@ -57,8 +57,11 @@ type Settings struct {
 	// minimum expiration field in the lease object. It is used for mixed-version
 	// compatibility.
 	MinExpirationSupported bool
-	// RangeLeaseDuration specifies the range lease duration.
+	// RangeLeaseDuration specifies the range lease duration. It's used for
+	// extending expiration leases.
 	RangeLeaseDuration time.Duration
+	// FortificationGracePeriod specifies the leader lease duration.
+	FortificationGracePeriod time.Duration
 }
 
 // PrevLeaseManipulation contains a set of instructions for manipulating the
@@ -537,7 +540,7 @@ func leaseMinTimestamp(st Settings, i BuildInput, nextType roachpb.LeaseType) hl
 		// there's no chance of an expiration regression. Still, it is still useful
 		// to set a minimum expiration time so that the new lease is guaranteed to
 		// have some validity period, even if the raft leader is unable to fortify.
-		minExp := i.Now.ToTimestamp().Add(int64(st.RangeLeaseDuration), 0)
+		minExp := i.Now.ToTimestamp().Add(int64(st.FortificationGracePeriod), 0)
 		minExp.Forward(i.PrevLeaseExpiration())
 		return minExp
 	default:

pkg/kv/kvserver/leases/build_test.go

@@ -26,10 +26,12 @@ var (
 	cts10 = hlc.ClockTimestamp{WallTime: 10}
 	cts20 = hlc.ClockTimestamp{WallTime: 20}
 	cts30 = hlc.ClockTimestamp{WallTime: 30}
+	cts35 = hlc.ClockTimestamp{WallTime: 35}
 	cts40 = hlc.ClockTimestamp{WallTime: 40}
 	cts50 = hlc.ClockTimestamp{WallTime: 50}
 	ts10  = cts10.ToTimestamp()
 	ts30  = cts30.ToTimestamp()
+	ts35  = cts35.ToTimestamp()
 	ts40  = cts40.ToTimestamp()
 	ts50  = cts50.ToTimestamp()
 )
@@ -203,6 +205,7 @@ func defaultSettings() Settings {
 		ExpToEpochEquiv:                   true,
 		MinExpirationSupported:            true,
 		RangeLeaseDuration:                20,
+		FortificationGracePeriod:          15,
 	}
 }
 
@@ -443,7 +446,7 @@ func TestBuild(t *testing.T) {
 						Term:            5,
 						Sequence:        8,
 						AcquisitionType: roachpb.LeaseAcquisitionType_Request,
-						MinExpiration:   ts40,
+						MinExpiration:   ts35,
 					},
 				},
 			},
@@ -460,7 +463,7 @@ func TestBuild(t *testing.T) {
 						Term:            5,
 						Sequence:        8,
 						AcquisitionType: roachpb.LeaseAcquisitionType_Request,
-						MinExpiration:   ts40,
+						MinExpiration:   ts35,
 					},
 				},
 			},
@@ -723,7 +726,7 @@ func TestBuild(t *testing.T) {
 						Term:            5,
 						Sequence:        7, // sequence not changed
 						AcquisitionType: roachpb.LeaseAcquisitionType_Request,
-						MinExpiration:   ts40,
+						MinExpiration:   ts35,
 					},
 				},
 			},
@@ -819,7 +822,7 @@ func TestBuild(t *testing.T) {
 						Term:            5,
 						Sequence:        7, // sequence not changed
 						AcquisitionType: roachpb.LeaseAcquisitionType_Request,
-						MinExpiration:   ts40,
+						MinExpiration:   ts35,
 					},
 				},
 			},
@@ -927,7 +930,7 @@ func TestBuild(t *testing.T) {
 						Term:            5,
 						Sequence:        8, // sequence changed
 						AcquisitionType: roachpb.LeaseAcquisitionType_Request,
-						MinExpiration:   ts40,
+						MinExpiration:   ts35,
 					},
 					PrevLeaseManipulation: PrevLeaseManipulation{
 						RevokeAndForwardNextExpiration: true,

pkg/kv/kvserver/replica_range_lease.go

@@ -796,8 +796,9 @@ func (r *Replica) leaseSettings(ctx context.Context) leases.Settings {
 		// TODO(arul): remove this field entirely.
 		ExpToEpochEquiv: true,
 		// TODO(radu): remove this field entirely.
-		MinExpirationSupported: true,
-		RangeLeaseDuration:     r.store.cfg.RangeLeaseDuration,
+		MinExpirationSupported:   true,
+		RangeLeaseDuration:       r.store.cfg.RangeLeaseDuration,
+		FortificationGracePeriod: r.store.cfg.FortificationGracePeriod,
 	}
 }