diff --git a/pkg/kv/kvserver/allocator/storepool/store_pool.go b/pkg/kv/kvserver/allocator/storepool/store_pool.go index ed2387fd220b..2b8dafb58e9b 100644 --- a/pkg/kv/kvserver/allocator/storepool/store_pool.go +++ b/pkg/kv/kvserver/allocator/storepool/store_pool.go @@ -1293,8 +1293,14 @@ func (sp *StorePool) getStoreListFromIDs( } var aliveStoreCount int - var throttled ThrottledStoreReasons - var storeDescriptors []roachpb.StoreDescriptor + // Both slices hold at most one entry per storeID, so pre-allocate to + // len(storeIDs) capacity. This avoids the repeated reallocate-and-copy as + // the slices grow: getStoreListFromIDs is on the allocator's hot path + // (called for every replication and rebalancing decision), where the + // incremental growth was observed to allocate ~1GB over a 5s profile and + // add GC pressure that hurts foreground latency. See #151952. + throttled := make(ThrottledStoreReasons, 0, len(storeIDs)) + storeDescriptors := make([]roachpb.StoreDescriptor, 0, len(storeIDs)) now := sp.clock.Now() timeUntilNodeDead := liveness.TimeUntilNodeDead.Get(&sp.st.SV) diff --git a/pkg/kv/kvserver/allocator/storepool/store_pool_test.go b/pkg/kv/kvserver/allocator/storepool/store_pool_test.go index 8461aa12320f..47d7d284d513 100644 --- a/pkg/kv/kvserver/allocator/storepool/store_pool_test.go +++ b/pkg/kv/kvserver/allocator/storepool/store_pool_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "github.com/cockroachdb/cockroach/pkg/gossip" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb" "github.com/cockroachdb/cockroach/pkg/roachpb" @@ -1005,3 +1006,47 @@ func TestStoreListString(t *testing.T) { " 5: ranges=50 leases=50 disk-usage=5.0 KiB queries-per-second=50.00 store-cpu-per-second=5µs io-overload=0.50(max=5.00)\n", MakeStoreList(stores).String()) } + +// BenchmarkGetStoreListFromIDs measures the per-call allocations of +// getStoreListFromIDs, which runs on the allocator's hot path. Run with +// -benchmem to observe allocs/op; pre-allocating the result slices to +// len(storeIDs) keeps it constant rather than growing with the store count. +func BenchmarkGetStoreListFromIDs(b *testing.B) { + defer leaktest.AfterTest(b)() + defer log.Scope(b).Close(b) + ctx := context.Background() + st := cluster.MakeTestingClusterSettings() + + const numStores = 50 + stopper, g, _, sp, mnl, _ := CreateTestStorePool(ctx, st, + liveness.TestTimeUntilNodeDead, true, /* deterministic */ + func() int { return numStores }, /* nodeCount */ + livenesspb.NodeLivenessStatus_DEAD) + defer stopper.Stop(ctx) + + stores := make([]*roachpb.StoreDescriptor, numStores) + storeIDs := make(roachpb.StoreIDSlice, numStores) + for i := range stores { + id := roachpb.StoreID(i + 1) + stores[i] = &roachpb.StoreDescriptor{ + StoreID: id, + Node: roachpb.NodeDescriptor{NodeID: roachpb.NodeID(i + 1)}, + } + storeIDs[i] = id + } + // Push the stores into gossip and mark every node live so all of them flow + // through the append path in getStoreListFromIDs. + for i, storeDesc := range stores { + if err := g.TestingAddInfoProtoAndWaitForAllCallbacks( + gossip.MakeStoreDescKey(storeDesc.StoreID), storeDesc, 0); err != nil { + b.Fatal(err) + } + mnl.SetNodeStatus(roachpb.NodeID(i+1), livenesspb.NodeLivenessStatus_LIVE) + } + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _, _ = sp.GetStoreListFromIDs(storeIDs, StoreFilterNone) + } +}