diff --git a/cmd/server/eviction_test.go b/cmd/server/eviction_test.go index d4090e90..66f8e883 100644 --- a/cmd/server/eviction_test.go +++ b/cmd/server/eviction_test.go @@ -541,13 +541,19 @@ func TestEstimateStoreTxBytes(t *testing.T) { PathJSON: `["aa","bb"]`, } est := estimateStoreTxBytes(tx) - // Verify the function returns a reasonable value matching our manual calculation + // Manual calculation: base + string lengths + index entries + perTxMaps + path hops + subpaths + hops := int64(len(txGetParsedPath(tx))) manualCalc := int64(storeTxBaseBytes) + int64(len(tx.RawHex)+len(tx.Hash)+len(tx.DecodedJSON)+len(tx.PathJSON)) + int64(numIndexesPerTx*indexEntryBytes) + manualCalc += perTxMapsBytes + manualCalc += hops * perPathHopBytes + if hops > 1 { + manualCalc += (hops * (hops - 1) / 2) * perSubpathEntryBytes + } if est != manualCalc { t.Fatalf("estimateStoreTxBytes = %d, want %d (manual calc)", est, manualCalc) } - if est < 600 || est > 800 { - t.Fatalf("estimateStoreTxBytes = %d, expected in range [600, 800]", est) + if est < 600 || est > 1200 { + t.Fatalf("estimateStoreTxBytes = %d, expected in range [600, 1200]", est) } } diff --git a/cmd/server/store.go b/cmd/server/store.go index 9ef65d22..d32d0ef6 100644 --- a/cmd/server/store.go +++ b/cmd/server/store.go @@ -1090,6 +1090,11 @@ func (s *PacketStore) GetPerfStoreStatsTyped() PerfPacketStoreStats { estimatedMB := math.Round(s.estimatedMemoryMB()*10) / 10 trackedMB := math.Round(s.trackedMemoryMB()*10) / 10 + var avgBytesPerPacket int64 + if totalLoaded > 0 { + avgBytesPerPacket = s.trackedBytes / int64(totalLoaded) + } + return PerfPacketStoreStats{ TotalLoaded: totalLoaded, TotalObservations: totalObs, @@ -1101,6 +1106,7 @@ func (s *PacketStore) GetPerfStoreStatsTyped() PerfPacketStoreStats { MaxPackets: 2386092, EstimatedMB: estimatedMB, TrackedMB: trackedMB, + AvgBytesPerPacket: avgBytesPerPacket, MaxMB: s.maxMemoryMB, Indexes: PacketStoreIndexes{ ByHash: hashIdx, @@ -2600,27 +2606,68 @@ func (s *PacketStore) buildDistanceIndex() { // These estimate the in-memory cost of StoreTx and StoreObs structs including // map/index overhead. They don't need to be exact — just proportional to actual // usage and independent of GC state. +// +// Issue #743: Previous estimates missed major per-packet allocations: +// - spTxIndex: O(path²) entries per tx (50-150MB at scale) +// - ResolvedPath on observations (~25MB at scale) +// - Per-tx maps: obsKeys, observerSet (~11MB at scale) +// - byPathHop index entries (20-40MB at scale) const ( storeTxBaseBytes = 384 // StoreTx struct fields + map headers + sync.Once + string headers storeObsBaseBytes = 192 // StoreObs struct fields + string headers indexEntryBytes = 48 // average cost of one index map entry (key + pointer + bucket overhead) numIndexesPerTx = 5 // byHash, byTxID, byNode, byPayloadType, nodeHashes entries numIndexesPerObs = 2 // byObsID, byObserver entries + + // Per-tx map overhead (obsKeys + observerSet): map header + initial buckets + perTxMapsBytes = 200 + + // Per path hop: byPathHop index entry (pointer + map bucket) + perPathHopBytes = 50 + + // Per subpath entry in spTxIndex: string key + slice append + pointer + perSubpathEntryBytes = 40 + + // Per resolved path element on an observation + perResolvedPathElemBytes = 24 // *string pointer + string header + avg pubkey length ) // estimateStoreTxBytes returns the estimated memory cost of a StoreTx (excluding observations). +// Includes per-tx maps (obsKeys, observerSet), byPathHop entries, and spTxIndex subpath entries. func estimateStoreTxBytes(tx *StoreTx) int64 { base := int64(storeTxBaseBytes) base += int64(len(tx.RawHex) + len(tx.Hash) + len(tx.DecodedJSON) + len(tx.PathJSON)) base += int64(numIndexesPerTx * indexEntryBytes) + + // Per-tx maps: obsKeys + observerSet + base += perTxMapsBytes + + // Path-dependent costs + hops := int64(len(txGetParsedPath(tx))) + base += hops * perPathHopBytes + + // spTxIndex: O(path²) subpath combinations + if hops > 1 { + subpaths := hops * (hops - 1) / 2 + base += subpaths * perSubpathEntryBytes + } + return base } // estimateStoreObsBytes returns the estimated memory cost of a StoreObs. +// Includes ResolvedPath slice overhead. func estimateStoreObsBytes(obs *StoreObs) int64 { base := int64(storeObsBaseBytes) base += int64(len(obs.PathJSON) + len(obs.ObserverID)) base += int64(numIndexesPerObs * indexEntryBytes) + + // ResolvedPath: slice header + per-element pointer/string + if obs.ResolvedPath != nil { + base += 24 // slice header + base += int64(len(obs.ResolvedPath)) * perResolvedPathElemBytes + } + return base } diff --git a/cmd/server/tracked_bytes_test.go b/cmd/server/tracked_bytes_test.go new file mode 100644 index 00000000..51b2e626 --- /dev/null +++ b/cmd/server/tracked_bytes_test.go @@ -0,0 +1,168 @@ +package main + +import ( + "testing" + "time" +) + +// TestEstimateStoreTxBytes_ReasonableValues verifies the estimate function +// returns reasonable values for different packet sizes. +func TestEstimateStoreTxBytes_ReasonableValues(t *testing.T) { + tx := &StoreTx{ + Hash: "abcdef1234567890", + RawHex: "deadbeef", + DecodedJSON: `{"type":"GRP_TXT"}`, + PathJSON: `["hop1","hop2","hop3"]`, + parsedPath: []string{"hop1", "hop2", "hop3"}, + pathParsed: true, + } + got := estimateStoreTxBytes(tx) + + // Should be at least base (384) + maps (200) + indexes + path/subpath costs + if got < 700 { + t.Errorf("estimate too low for 3-hop tx: %d", got) + } + if got > 5000 { + t.Errorf("estimate unreasonably high for 3-hop tx: %d", got) + } +} + +// TestEstimateStoreTxBytes_ManyHopsSubpaths verifies that packets with many +// hops estimate significantly more due to O(path²) subpath index entries. +func TestEstimateStoreTxBytes_ManyHopsSubpaths(t *testing.T) { + tx2 := &StoreTx{ + Hash: "aabb", + parsedPath: []string{"a", "b"}, + pathParsed: true, + } + tx10 := &StoreTx{ + Hash: "aabb", + parsedPath: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}, + pathParsed: true, + } + est2 := estimateStoreTxBytes(tx2) + est10 := estimateStoreTxBytes(tx10) + + // 10 hops → 45 subpath combos × 40 = 1800 bytes just for subpaths + if est10 <= est2 { + t.Errorf("10-hop (%d) should estimate more than 2-hop (%d)", est10, est2) + } + if est10 < est2+1500 { + t.Errorf("10-hop (%d) should estimate at least 1500 more than 2-hop (%d)", est10, est2) + } +} + +// TestEstimateStoreObsBytes_WithResolvedPath verifies that observations with +// ResolvedPath estimate more than those without. +func TestEstimateStoreObsBytes_WithResolvedPath(t *testing.T) { + s1, s2, s3 := "node1", "node2", "node3" + + obsNoRP := &StoreObs{ + ObserverID: "obs1", + PathJSON: `["a","b"]`, + } + obsWithRP := &StoreObs{ + ObserverID: "obs1", + PathJSON: `["a","b"]`, + ResolvedPath: []*string{&s1, &s2, &s3}, + } + + estNo := estimateStoreObsBytes(obsNoRP) + estWith := estimateStoreObsBytes(obsWithRP) + + if estWith <= estNo { + t.Errorf("obs with ResolvedPath (%d) should estimate more than without (%d)", estWith, estNo) + } +} + +// TestEstimateStoreObsBytes_ManyObservations verifies that 15 observations +// estimate significantly more than 1. +func TestEstimateStoreObsBytes_ManyObservations(t *testing.T) { + est1 := estimateStoreObsBytes(&StoreObs{ObserverID: "a", PathJSON: `["x"]`}) + est15 := int64(0) + for i := 0; i < 15; i++ { + est15 += estimateStoreObsBytes(&StoreObs{ObserverID: "a", PathJSON: `["x"]`}) + } + if est15 <= est1*10 { + t.Errorf("15 obs total (%d) should be >10x single obs (%d)", est15, est1) + } +} + +// TestTrackedBytesMatchesSumAfterInsert verifies that trackedBytes equals the +// sum of individual estimates after inserting packets via makeTestStore. +func TestTrackedBytesMatchesSumAfterInsert(t *testing.T) { + store := makeTestStore(20, time.Now().Add(-2*time.Hour), 5) + + // Manually compute trackedBytes as sum of estimates + var expectedSum int64 + for _, tx := range store.packets { + expectedSum += estimateStoreTxBytes(tx) + for _, obs := range tx.Observations { + expectedSum += estimateStoreObsBytes(obs) + } + } + + if store.trackedBytes != expectedSum { + t.Errorf("trackedBytes=%d, expected sum=%d", store.trackedBytes, expectedSum) + } +} + +// TestEvictionTriggersWithImprovedEstimates verifies that eviction triggers +// at the right point with the improved (higher) estimates. +func TestEvictionTriggersWithImprovedEstimates(t *testing.T) { + store := makeTestStore(100, time.Now().Add(-10*time.Hour), 5) + + // trackedBytes for 100 packets is small — artificially set maxMemoryMB + // so highWatermark is just below trackedBytes to trigger eviction. + highWatermarkBytes := store.trackedBytes - 1000 + if highWatermarkBytes < 1 { + highWatermarkBytes = 1 + } + // maxMemoryMB * 1048576 = highWatermark, so maxMemoryMB = ceil(highWatermarkBytes / 1048576) + // But that'll be 0 for small values. Instead, directly set trackedBytes high. + store.trackedBytes = 6 * 1048576 // 6MB + store.maxMemoryMB = 3 // 3MB limit + + beforeCount := len(store.packets) + store.RunEviction() + afterCount := len(store.packets) + + if afterCount >= beforeCount { + t.Errorf("expected eviction to remove packets: before=%d, after=%d, trackedBytes=%d, maxMB=%d", + beforeCount, afterCount, store.trackedBytes, store.maxMemoryMB) + } + // trackedBytes should have decreased + if store.trackedBytes >= 6*1048576 { + t.Errorf("trackedBytes should have decreased after eviction") + } +} + +// BenchmarkEstimateStoreTxBytes verifies the estimate function is fast. +func BenchmarkEstimateStoreTxBytes(b *testing.B) { + tx := &StoreTx{ + Hash: "abcdef1234567890", + RawHex: "deadbeefdeadbeef", + DecodedJSON: `{"type":"GRP_TXT","payload":"hello"}`, + PathJSON: `["hop1","hop2","hop3","hop4","hop5"]`, + parsedPath: []string{"hop1", "hop2", "hop3", "hop4", "hop5"}, + pathParsed: true, + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + estimateStoreTxBytes(tx) + } +} + +// BenchmarkEstimateStoreObsBytes verifies the obs estimate function is fast. +func BenchmarkEstimateStoreObsBytes(b *testing.B) { + s := "resolvedNodePubkey123456" + obs := &StoreObs{ + ObserverID: "observer1234", + PathJSON: `["a","b","c"]`, + ResolvedPath: []*string{&s, &s, &s}, + } + b.ResetTimer() + for i := 0; i < b.N; i++ { + estimateStoreObsBytes(obs) + } +} diff --git a/cmd/server/types.go b/cmd/server/types.go index 51af55a3..24769df0 100644 --- a/cmd/server/types.go +++ b/cmd/server/types.go @@ -176,6 +176,7 @@ type PerfPacketStoreStats struct { MaxPackets int `json:"maxPackets"` EstimatedMB float64 `json:"estimatedMB"` TrackedMB float64 `json:"trackedMB"` + AvgBytesPerPacket int64 `json:"avgBytesPerPacket"` MaxMB int `json:"maxMB"` Indexes PacketStoreIndexes `json:"indexes"` }