kvserver: compute MVCCStats in AdminSplit instead of splitTrigger

Previously, we computed MVCC stats for the LHS and RHS of a split in
splitTrigger, while holding latches. This resulted in high tail
latencies due to foreground work not being able to proceed as long as
the split holds these latches. More details in cockroachdb#119499.

This change moves some of the stats computation to AdminSplit, where no
latches are held. In particular, in AdminSplit, we pre-compute the part
of the LHS stats corresponding to user-data spans, and pass that to the
splitTrigger. In the splitTrigger, we iterate only over the non-user
spans in the range (which are small), and combine the resulting stats
with the passed in user stats to arrive at the new LHS stats. The RHS
stats are derived by substituting the LHS stats from the total on-disk
stats. This computation does not produce 100% correct stats because any
writes concurrent with the split may not be attributed correctly to the
LHS or RHS. Therefore, both LHS and RHS stats are marked with
ContainsEstimates.

To prevent the stats from drifting after successive splits, we kick off
a stats re-computation in AdminSplit (only if the on-disk stats contain
estimates). Thus, each split may introduce some stats estimates, but
before doing so it will correct any previous estimates.

There are two invariants that the stats computation guarantees: 1.
Non-user stats (SysBytes, SysCount, etc.) are always correct. This is
because we scan those spans while holding latches. 2. If there are no
writes concurrent with the split, the stats are always correct.

Fixes: cockroachdb#119499

Release note (performance improvement): Splits no longer
hold latches for time proportional to the range size while computing
MVCC stats. Instead, MVCC stats are pre-computed before the critical
section of the split. As a side effect, the resulting stats are no
longer 100% accurate because they may correctly distribute writes
concurrent with the split. To mitigate this, and to prevent the stats
from drifting after successive splits, the existing stored stats are
re-computed and corrected (if needed) as part of the non-critical
section of the split.

docs/generated/settings/settings-for-tenants.txt

@@ -337,4 +337,4 @@ trace.snapshot.rate	duration	0s	if non-zero, interval at which background trace
 trace.span_registry.enabled	boolean	true	if set, ongoing traces can be seen at https://<ui>/#/debug/tracez	application
 trace.zipkin.collector	string		the address of a Zipkin instance to receive traces, as <host>:<port>. If no port is specified, 9411 will be used.	application
 ui.display_timezone	enumeration	etc/utc	the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]	application
-version	version	1000023.2-upgrading-to-1000024.1-step-018	set the active cluster version in the format '<major>.<minor>'	application
+version	version	1000023.2-upgrading-to-1000024.1-step-020	set the active cluster version in the format '<major>.<minor>'	application

docs/generated/settings/settings.html

@@ -291,6 +291,6 @@
 <tr><td><div id="setting-trace-span-registry-enabled" class="anchored"><code>trace.span_registry.enabled</code></div></td><td>boolean</td><td><code>true</code></td><td>if set, ongoing traces can be seen at https://&lt;ui&gt;/#/debug/tracez</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-trace-zipkin-collector" class="anchored"><code>trace.zipkin.collector</code></div></td><td>string</td><td><code></code></td><td>the address of a Zipkin instance to receive traces, as &lt;host&gt;:&lt;port&gt;. If no port is specified, 9411 will be used.</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 <tr><td><div id="setting-ui-display-timezone" class="anchored"><code>ui.display_timezone</code></div></td><td>enumeration</td><td><code>etc/utc</code></td><td>the timezone used to format timestamps in the ui [etc/utc = 0, america/new_york = 1]</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
-<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.2-upgrading-to-1000024.1-step-018</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
+<tr><td><div id="setting-version" class="anchored"><code>version</code></div></td><td>version</td><td><code>1000023.2-upgrading-to-1000024.1-step-020</code></td><td>set the active cluster version in the format &#39;&lt;major&gt;.&lt;minor&gt;&#39;</td><td>Serverless/Dedicated/Self-Hosted</td></tr>
 </tbody>
 </table>

pkg/clusterversion/cockroach_versions.go

@@ -296,6 +296,10 @@ const (
 	// database to be SURVIVE ZONE.
 	V24_1_SystemDatabaseSurvivability
 
+	// V24_1_EstimatedMVCCStatsInSplit introduces MVCC stats estimates during range
+	// splits.
+	V24_1_EstimatedMVCCStatsInSplit
+
 	numKeys
 )
 
@@ -361,6 +365,7 @@ var versionTable = [numKeys]roachpb.Version{
 	V24_1_SessionBasedLeasingUpgradeDescriptor: {Major: 23, Minor: 2, Internal: 14},
 	V24_1_PebbleFormatSyntheticPrefixSuffix:    {Major: 23, Minor: 2, Internal: 16},
 	V24_1_SystemDatabaseSurvivability:          {Major: 23, Minor: 2, Internal: 18},
+	V24_1_EstimatedMVCCStatsInSplit:            {Major: 23, Minor: 2, Internal: 20},
 }
 
 // Latest is always the highest version key. This is the maximum logical cluster

pkg/kv/kvserver/batcheval/cmd_end_transaction.go

@@ -30,6 +30,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/spanset"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/stateloader"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/storage"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
 	"github.com/cockroachdb/cockroach/pkg/util"
@@ -40,6 +41,29 @@ import (
 	"github.com/cockroachdb/redact"
 )
 
+// EnableEstimatedMVCCStatsInSplit controls whether the MVCC stats produced
+// during splits are estimated (when enabled) or 100% accurate (when disabled).
+// Defaults to true but metamorphically changed in tests.
+var EnableEstimatedMVCCStatsInSplit = settings.RegisterBoolSetting(
+	settings.SystemVisible,
+	"kv.split.estimated_mvcc_stats.enabled",
+	"if enabled, MVCC stats will be computed estimated (as opposed to "+
+		"computed accurately) during splits",
+	util.ConstantWithMetamorphicTestBool("kv.split.estimated_mvcc_stats.enabled", true))
+
+// EnableMVCCStatsRecomputationInSplit controls whether the MVCC stats for a
+// range are re-computed at the beginning of the split (in AdminSplit). Doing so
+// prevents stats estimates by successive splits from drifting too much from the
+// real stats. However, re-computation is expensive, so it can be disabled using
+// this setting if it's causing performance issues.
+// Defaults to true but metamorphically changed in tests.
+var EnableMVCCStatsRecomputationInSplit = settings.RegisterBoolSetting(
+	settings.SystemVisible,
+	"kv.split.mvcc_stats_recomputation.enabled",
+	"if enabled, MVCC stats will be recomputed at the beginning of a split "+
+		"to prevent stats estimates from drifting",
+	util.ConstantWithMetamorphicTestBool("kv.split.mvcc_stats_recomputation.enabled", true))
+
 func init() {
 	RegisterReadWriteCommand(kvpb.EndTxn, declareKeysEndTxn, EndTxn)
 }
@@ -1032,6 +1056,22 @@ func splitTrigger(
 			"unable to determine whether right hand side of split is empty")
 	}
 
+	// The intentInterleavingIterator doesn't like iterating over spans containing
+	// both local and global keys. Here we only care about global keys, so if the
+	// LHS includes some local keys, move the start key to right after LocalMax.
+	startKey := split.LeftDesc.StartKey.AsRawKey()
+	endKey := split.LeftDesc.EndKey.AsRawKey()
+	if startKey.Compare(keys.LocalMax) <= 0 && endKey.Compare(keys.LocalMax) > 0 {
+		startKey = keys.LocalMax.Next()
+	}
+	emptyLHS, err := storage.MVCCIsSpanEmpty(ctx, batch, storage.MVCCIsSpanEmptyOptions{
+		StartKey: startKey, EndKey: endKey,
+	})
+	if err != nil {
+		return enginepb.MVCCStats{}, result.Result{}, errors.Wrapf(err,
+			"unable to determine whether left hand side of split is empty")
+	}
+
 	rangeKeyDeltaMS, err := computeSplitRangeKeyStatsDelta(ctx, batch, split.LeftDesc, split.RightDesc)
 	if err != nil {
 		return enginepb.MVCCStats{}, result.Result{}, errors.Wrap(err,
@@ -1055,12 +1095,16 @@ func splitTrigger(
 	}
 
 	h := splitStatsHelperInput{
-		AbsPreSplitBothStored: currentStats,
-		DeltaBatchEstimated:   bothDeltaMS,
-		DeltaRangeKey:         rangeKeyDeltaMS,
-		PostSplitScanLeftFn:   makeScanStatsFn(ctx, batch, ts, &split.LeftDesc, "left hand side"),
-		PostSplitScanRightFn:  makeScanStatsFn(ctx, batch, ts, &split.RightDesc, "right hand side"),
-		ScanRightFirst:        splitScansRightForStatsFirst || emptyRHS,
+		AbsPreSplitBothStored:    currentStats,
+		DeltaBatchEstimated:      bothDeltaMS,
+		DeltaRangeKey:            rangeKeyDeltaMS,
+		PostSplitScanLeftFn:      makeScanStatsFn(ctx, batch, ts, &split.LeftDesc, "left hand side", false /* excludeUserSpans */),
+		PostSplitScanRightFn:     makeScanStatsFn(ctx, batch, ts, &split.RightDesc, "right hand side", false /* excludeUserSpans */),
+		ScanRightFirst:           splitScansRightForStatsFirst || emptyRHS,
+		LeftIsEmpty:              emptyLHS,
+		RightIsEmpty:             emptyRHS,
+		PreSplitLeftUser:         split.PreSplitLeftUserStats,
+		PostSplitScanLocalLeftFn: makeScanStatsFn(ctx, batch, ts, &split.LeftDesc, "local left hand side", true /* excludeUserSpans */),
 	}
 	return splitTriggerHelper(ctx, rec, batch, h, split, ts)
 }
@@ -1081,9 +1125,14 @@ func makeScanStatsFn(
 	ts hlc.Timestamp,
 	sideDesc *roachpb.RangeDescriptor,
 	sideName string,
+	excludeUserSpans bool,
 ) splitStatsScanFn {
+	computeStatsFn := rditer.ComputeStatsForRange
+	if excludeUserSpans {
+		computeStatsFn = rditer.ComputeStatsForRangeExcludingUser
+	}
 	return func() (enginepb.MVCCStats, error) {
-		sideMS, err := rditer.ComputeStatsForRange(ctx, sideDesc, reader, ts.WallTime)
+		sideMS, err := computeStatsFn(ctx, sideDesc, reader, ts.WallTime)
 		if err != nil {
 			return enginepb.MVCCStats{}, errors.Wrapf(err,
 				"unable to compute stats for %s range after split", sideName)
@@ -1128,7 +1177,16 @@ func splitTriggerHelper(
 	// modifications to the left hand side are allowed after this line and any
 	// modifications to the right hand side are accounted for by updating the
 	// helper's AbsPostSplitRight() reference.
-	h, err := makeSplitStatsHelper(statsInput)
+	var h splitStatsHelper
+	// If the leaseholder node is running an older version, it would not include
+	// the PreSplitLeftUser proto field, which is needed in
+	// makeEstimatedSplitStatsHelper.
+	if rec.ClusterSettings().Version.IsActive(ctx, clusterversion.V24_1_EstimatedMVCCStatsInSplit) &&
+		EnableEstimatedMVCCStatsInSplit.Get(&rec.ClusterSettings().SV) {
+		h, err = makeEstimatedSplitStatsHelper(statsInput)
+	} else {
+		h, err = makeSplitStatsHelper(statsInput)
+	}
 	if err != nil {
 		return enginepb.MVCCStats{}, result.Result{}, err
 	}

pkg/kv/kvserver/batcheval/split_stats_helper.go

@@ -133,6 +133,24 @@ type splitStatsHelperInput struct {
 	// input stats contain estimates, this is the only side that needs to
 	// be scanned.
 	ScanRightFirst bool
+	// LeftIsEmpty denotes that the left hand side is empty. If this is the case,
+	// the entire left hand side will be scanned to compute stats accurately. This
+	// is cheap because the range is empty.
+	LeftIsEmpty bool
+	// RightIsEmpty denotes that the right hand side is empty. If this is the
+	// case, the entire right hand side will be scanned to compute stats
+	// accurately. This is cheap because the range is empty.
+	RightIsEmpty bool
+	// PreSplitLeftUser contains the pre-split user-only stats of the left hand
+	// side. Those are expensive to compute, so we compute them in AdminSplit,
+	// before holding latches, and pass them to splitTrigger.
+	PreSplitLeftUser enginepb.MVCCStats
+	// PostSplitScanLocalLeftFn returns the stats for the non-user spans of the
+	// left hand side. Adding these to PreSplitLeftUser results in an estimate of
+	// the left hand side stats. It's only an estimate because any writes to the
+	// user spans of the left hand side, concurrent with the split, may not be
+	// accounted for.
+	PostSplitScanLocalLeftFn splitStatsScanFn
 }
 
 // makeSplitStatsHelper initializes a splitStatsHelper. The values in the input
@@ -194,6 +212,89 @@ func makeSplitStatsHelper(input splitStatsHelperInput) (splitStatsHelper, error)
 	return h, nil
 }
 
+// makeEstimatedSplitStatsHelper is like makeSplitStatsHelper except it does
+// not scan the entire LHS or RHS by calling PostSplitScan(Left|Right)Fn().
+// Instead, this function derives the post-split LHS stats by adding
+// the pre-split user LHS stats to the non-user LHS stats computed here by
+// calling PostSplitScanLocalLeftFn(). The resulting stats are not guaranteed
+// to be 100% accurate, so they are marked with ContainsEstimates. However,
+// if there are no writes concurrent with the split (i.e. the pre-computed
+// PreSplitLeftUser still correspond to the correct LHS user stats), then the
+// stats are guaranteed to be correct.
+// INVARIANT 1: Non-user stats (SysBytes, SysCount, etc.) are always correct.
+// This is because we scan those spans while holding latches in this function.
+// INVARIANT 2: If there are no writes concurrent with the split, the stats are
+// always correct.
+func makeEstimatedSplitStatsHelper(input splitStatsHelperInput) (splitStatsHelper, error) {
+	h := splitStatsHelper{
+		in: input,
+	}
+
+	// Fast path: if both ranges are guaranteed to be empty, just scan them and
+	// return early. They do not contain estimates.
+	if h.in.LeftIsEmpty && h.in.RightIsEmpty {
+		leftStats, err := input.PostSplitScanLeftFn()
+		if err != nil {
+			return splitStatsHelper{}, err
+		}
+		h.absPostSplitLeft = &leftStats
+		rightStats, err := input.PostSplitScanRightFn()
+		if err != nil {
+			return splitStatsHelper{}, err
+		}
+		h.absPostSplitRight = &rightStats
+		return h, nil
+	}
+
+	var absPostSplitFirst enginepb.MVCCStats
+	var err error
+	// If either the RHS or LHS is empty, scan it (it's cheap).
+	if h.in.RightIsEmpty {
+		absPostSplitFirst, err = input.PostSplitScanRightFn()
+		h.absPostSplitRight = &absPostSplitFirst
+	} else if h.in.LeftIsEmpty {
+		absPostSplitFirst, err = input.PostSplitScanLeftFn()
+		h.absPostSplitLeft = &absPostSplitFirst
+	} else {
+		// Otherwise, compute the LHS stats by adding the user-only LHS stats passed
+		// into the split trigger and the non-user LHS stats computed by calling
+		// PostSplitScanLocalLeftFn(). The local key space is very small, so this is
+		// not expensive. Scanning the local key space ensures that any writes to
+		// local keys during the split (e.g. to range descriptors and transaction
+		// records) are accounted for.
+		absPostSplitFirst, err = input.PostSplitScanLocalLeftFn()
+		absPostSplitFirst.Add(input.PreSplitLeftUser)
+		h.absPostSplitLeft = &absPostSplitFirst
+	}
+	if err != nil {
+		return splitStatsHelper{}, err
+	}
+
+	// For the second side of the split, the computation is identical to that in
+	// makeSplitStatsHelper: subtract the first side's stats from the total, and
+	// adjust DeltaBatchEstimated and DeltaRangeKey.
+	ms := h.in.AbsPreSplitBothStored
+	ms.Subtract(absPostSplitFirst)
+	ms.Add(h.in.DeltaBatchEstimated)
+	ms.Add(h.in.DeltaRangeKey)
+	if h.in.RightIsEmpty {
+		h.absPostSplitLeft = &ms
+	} else {
+		h.absPostSplitRight = &ms
+	}
+
+	// Mark the new stats with ContainsEstimates only if the corresponding range
+	// is not empty.
+	if !h.in.LeftIsEmpty {
+		h.absPostSplitLeft.ContainsEstimates++
+	}
+	if !h.in.RightIsEmpty {
+		h.absPostSplitRight.ContainsEstimates++
+	}
+
+	return h, nil
+}
+
 // AbsPostSplitRight returns the stats of the right hand side created by the
 // split. The result is returned as a pointer because the caller can freely
 // modify it, assuming they're adding only stats corresponding to mutations that

pkg/kv/kvserver/client_raft_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/allocator/storepool"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowdispatch"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
@@ -5811,7 +5812,11 @@ func TestRaftSnapshotsWithMVCCRangeKeys(t *testing.T) {
 		require.Len(t, ccResp.Result, 1)
 		result := ccResp.Result[0]
 		require.Equal(t, desc.RangeID, result.RangeID)
-		require.Equal(t, kvpb.CheckConsistencyResponse_RANGE_CONSISTENT, result.Status, "%+v", result)
+		if batcheval.EnableEstimatedMVCCStatsInSplit.Get(&ts.ClusterSettings().SV) {
+			require.Equal(t, kvpb.CheckConsistencyResponse_RANGE_CONSISTENT_STATS_ESTIMATED, result.Status, "%+v", result)
+		} else {
+			require.Equal(t, kvpb.CheckConsistencyResponse_RANGE_CONSISTENT, result.Status, "%+v", result)
+		}
 	}
 
 	checkConsistency(descA)

pkg/kv/kvserver/client_split_test.go

@@ -34,6 +34,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv/kvpb"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/abortspan"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/batcheval"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/isolation"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/gc"
@@ -851,9 +852,16 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
 	require.GreaterOrEqual(t, msLeft.RangeValBytes+msRight.RangeValBytes, ms.RangeValBytes)
 	require.GreaterOrEqual(t, msLeft.GCBytesAge+msRight.GCBytesAge, ms.GCBytesAge)
 
-	// Stats should agree with recomputation.
+	// Stats should agree with re-computation.
 	assertRecomputedStats(t, "LHS after split", snap, repl.Desc(), msLeft, s.Clock().PhysicalNow())
 	assertRecomputedStats(t, "RHS after split", snap, replRight.Desc(), msRight, s.Clock().PhysicalNow())
+	if batcheval.EnableEstimatedMVCCStatsInSplit.Get(&store.ClusterSettings().SV) {
+		// The LHS gets ContainsEstimates = 1 from the split, which is then multiplied
+		// by 2 in evaluateProposal for migration reasons.
+		require.Equal(t, msLeft.ContainsEstimates, int64(2))
+		// The RHS gets ContainsEstimates = 1 from the split.
+		require.Equal(t, msRight.ContainsEstimates, int64(1))
+	}
 
 	// Merge the ranges back together, and assert that the merged stats
 	// agree with the pre-split stats.
@@ -871,6 +879,13 @@ func TestStoreRangeSplitMergeStats(t *testing.T) {
 
 	msMerged.SysBytes, msMerged.SysCount, msMerged.AbortSpanBytes = 0, 0, 0
 	ms.AgeTo(msMerged.LastUpdateNanos)
+	if batcheval.EnableEstimatedMVCCStatsInSplit.Get(&store.ClusterSettings().SV) {
+		// The stats delta accumulated by the merge has ContainsEstimates = 1 (from the
+		// RHS stats). It gets multiplied by 2 in evaluateProposal for migration
+		// reasons, and then added to the LHS stats, which has ContainsEstimates = 2
+		// from above.
+		ms.ContainsEstimates = 4
+	}
 	require.Equal(t, ms, msMerged, "post-merge stats differ from pre-split")
 }
 
@@ -958,10 +973,56 @@ func TestStoreRangeSplitWithConcurrentWrites(t *testing.T) {
 	rhsRepl := store.LookupReplica(splitKeyAddr)
 	rhsStats, err := stateloader.Make(rhsRepl.RangeID).LoadMVCCStats(ctx, snap)
 	require.NoError(t, err)
+	if batcheval.EnableEstimatedMVCCStatsInSplit.Get(&store.ClusterSettings().SV) {
+		require.Greater(t, lhsStats.ContainsEstimates, int64(0))
+		require.Greater(t, rhsStats.ContainsEstimates, int64(0))
+	} else {
+		// Stats should agree with re-computation.
+		assertRecomputedStats(t, "LHS after split", snap, lhsRepl.Desc(), lhsStats, s.Clock().PhysicalNow())
+		assertRecomputedStats(t, "RHS after split", snap, rhsRepl.Desc(), rhsStats, s.Clock().PhysicalNow())
+	}
+
+	// If we used estimated stats while splitting the range, the stats on disk
+	// will not match the stats recomputed from the range. We expect both of the
+	// concurrent writes to be attributed to the RHS (instead of 1 to the LHS and
+	// 1 th the RHS). But if we split these ranges one more time (no concurrent
+	// writes this time), we expect the stats to be corrected and not drift.
+	splitKeyLeft := roachpb.Key("aa")
+	splitKeyLeftAddr, err := keys.Addr(splitKeyLeft)
+	require.NoError(t, err)
+	_, pErr = kv.SendWrapped(ctx, store.TestSender(), adminSplitArgs(splitKeyLeft))
+	require.NoError(t, pErr.GoError())
+	splitKeyRight := roachpb.Key("bb")
+	splitKeyRightAddr, err := keys.Addr(splitKeyRight)
+	require.NoError(t, err)
+	_, pErr = kv.SendWrapped(ctx, store.TestSender(), adminSplitArgs(splitKeyRight))
+	require.NoError(t, pErr.GoError())
+
+	snap = store.TODOEngine().NewSnapshot()
+	defer snap.Close()
+	lhs1Stats, err := stateloader.Make(lhsRepl.RangeID).LoadMVCCStats(ctx, snap)
+	require.NoError(t, err)
+	lhs2Repl := store.LookupReplica(splitKeyLeftAddr)
+	lhs2Stats, err := stateloader.Make(lhs2Repl.RangeID).LoadMVCCStats(ctx, snap)
+	require.NoError(t, err)
+	rhs1Stats, err := stateloader.Make(rhsRepl.RangeID).LoadMVCCStats(ctx, snap)
+	require.NoError(t, err)
+	rhs2Repl := store.LookupReplica(splitKeyRightAddr)
+	rhs2Stats, err := stateloader.Make(rhs2Repl.RangeID).LoadMVCCStats(ctx, snap)
+	require.NoError(t, err)
 
 	// Stats should agree with re-computation.
-	assertRecomputedStats(t, "LHS after split", snap, lhsRepl.Desc(), lhsStats, s.Clock().PhysicalNow())
-	assertRecomputedStats(t, "RHS after split", snap, rhsRepl.Desc(), rhsStats, s.Clock().PhysicalNow())
+	assertRecomputedStats(t, "LHS1 after second split", snap, lhsRepl.Desc(), lhs1Stats, s.Clock().PhysicalNow())
+	assertRecomputedStats(t, "LHS2 after second split", snap, lhs2Repl.Desc(), lhs2Stats, s.Clock().PhysicalNow())
+	assertRecomputedStats(t, "RHS1 after second split", snap, rhsRepl.Desc(), rhs1Stats, s.Clock().PhysicalNow())
+	assertRecomputedStats(t, "RHS2 after second split", snap, rhs2Repl.Desc(), rhs2Stats, s.Clock().PhysicalNow())
+	if batcheval.EnableEstimatedMVCCStatsInSplit.Get(&store.ClusterSettings().SV) {
+		require.Greater(t, lhs1Stats.ContainsEstimates, int64(0))
+		require.Greater(t, lhs2Stats.ContainsEstimates, int64(0))
+		// This range is empty, so we don't label it with ContainsEstimates.
+		require.Equal(t, int64(0), rhs1Stats.ContainsEstimates)
+		require.Greater(t, rhs2Stats.ContainsEstimates, int64(0))
+	}
 }
 
 // RaftMessageHandlerInterceptor wraps a storage.IncomingRaftMessageHandler. It