From 972993980eb62bd55daeea3b138a9c9d449e5e87 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Tue, 3 Nov 2026 01:46:11 +0000 Subject: [PATCH] refactor(session): remove unreachable ChunkManager eviction code ChunkManager.sessionMap is never written in this package, so the limit and emergency eviction paths cannot run. Remove enforceSessionLimit, findOldestSessions, CanCreateSession, EmergencyCleanup, GetSessionCount, GetMemoryStats and globalMaxSessions, with the tests that only covered them. The session/chunking package keeps its own copy. --- coverage_boost_final_test.go | 79 +------------ session_chunk_manager.go | 223 ----------------------------------- 2 files changed, 1 insertion(+), 301 deletions(-) diff --git a/coverage_boost_final_test.go b/coverage_boost_final_test.go index 820bb60..93fbe48 100644 --- a/coverage_boost_final_test.go +++ b/coverage_boost_final_test.go @@ -921,82 +921,6 @@ func TestMemoryTestOrchestrator_StopLeakDetection_CoverageBoost(t *testing.T) { // CHUNK MANAGER TESTS // ============================================================================= -func TestChunkManager_GetSessionCount_CoverageBoost(t *testing.T) { - logger := NewLogger("debug") - cm := NewChunkManager(logger) - if cm == nil { - t.Fatal("Expected non-nil ChunkManager") - } - defer cm.Shutdown() - - // Test GetSessionCount - count := cm.GetSessionCount() - if count != 0 { - t.Errorf("Expected 0 sessions, got %d", count) - } -} - -func TestChunkManager_GetMemoryStats_CoverageBoost(t *testing.T) { - logger := NewLogger("debug") - cm := NewChunkManager(logger) - if cm == nil { - t.Fatal("Expected non-nil ChunkManager") - } - defer cm.Shutdown() - - // Test GetMemoryStats - stats := cm.GetMemoryStats() - if stats == nil { - t.Fatal("Expected non-nil stats") - } - - // Verify expected keys exist - if _, ok := stats["active_sessions"]; !ok { - t.Error("Expected active_sessions key in stats") - } - if _, ok := stats["max_sessions"]; !ok { - t.Error("Expected max_sessions key in stats") - } - if _, ok := stats["bytes_allocated"]; !ok { - t.Error("Expected bytes_allocated key in stats") - } -} - -func TestChunkManager_CanCreateSession_CoverageBoost(t *testing.T) { - logger := NewLogger("debug") - cm := NewChunkManager(logger) - if cm == nil { - t.Fatal("Expected non-nil ChunkManager") - } - defer cm.Shutdown() - - // Test CanCreateSession - should be true initially - canCreate, err := cm.CanCreateSession() - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if !canCreate { - t.Error("Expected CanCreateSession to return true when empty") - } -} - -func TestChunkManager_EmergencyCleanup_CoverageBoost(t *testing.T) { - logger := NewLogger("debug") - cm := NewChunkManager(logger) - if cm == nil { - t.Fatal("Expected non-nil ChunkManager") - } - defer cm.Shutdown() - - // Test EmergencyCleanup - should not panic on empty session map - cm.EmergencyCleanup() - - // Verify no sessions exist - if cm.GetSessionCount() != 0 { - t.Error("Expected 0 sessions after cleanup") - } -} - func TestChunkManager_CleanupExpiredSessions_CoverageBoost(t *testing.T) { logger := NewLogger("debug") cm := NewChunkManager(logger) @@ -1008,8 +932,7 @@ func TestChunkManager_CleanupExpiredSessions_CoverageBoost(t *testing.T) { // Test CleanupExpiredSessions - should not panic on empty session map cm.CleanupExpiredSessions() - // Verify no sessions exist - if cm.GetSessionCount() != 0 { + if len(cm.sessionMap) != 0 { t.Error("Expected 0 sessions after cleanup") } } diff --git a/session_chunk_manager.go b/session_chunk_manager.go index 1fbd1e9..efe3c9d 100644 --- a/session_chunk_manager.go +++ b/session_chunk_manager.go @@ -32,7 +32,6 @@ type TokenConfig struct { // Global session tracking to prevent memory leaks across all instances var ( globalSessionCount int64 = 0 - globalMaxSessions int64 = 5000 // CRITICAL FIX: Global limit of 5000 total sessions ) // ResetGlobalSessionCounters resets global session tracking for testing @@ -1243,226 +1242,4 @@ func (cm *ChunkManager) CleanupExpiredSessions(force ...bool) { cm.logger.Debugf("Cleaned up %d expired sessions, freed %d bytes", len(expiredKeys), totalBytesFreed) } - - // Enforce max sessions limit - if len(cm.sessionMap) > cm.maxSessions { - cm.enforceSessionLimit() - } -} - -// enforceSessionLimit removes oldest sessions when limit is exceeded -// Uses partial sort (O(n) for finding k smallest) instead of full sort (O(n log n)) -func (cm *ChunkManager) enforceSessionLimit() { - currentLocal := len(cm.sessionMap) - currentGlobal := atomic.LoadInt64(&globalSessionCount) - - // CRITICAL FIX: Aggressive eviction when approaching limits - shouldEvict := false - targetCapacity := cm.maxSessions - - // Check global limit first (more critical) - if currentGlobal >= globalMaxSessions { - shouldEvict = true - targetCapacity = cm.maxSessions / 4 // Aggressive reduction to 25% - } else if currentGlobal >= globalMaxSessions*8/10 { // 80% of global - shouldEvict = true - targetCapacity = cm.maxSessions / 2 // Reduce to 50% - } else if currentLocal >= cm.maxSessions { - shouldEvict = true - targetCapacity = cm.maxSessions * 3 / 4 // Reduce to 75% - } - - if !shouldEvict { - return - } - - // Calculate how many sessions to remove - excessCount := currentLocal - targetCapacity - if excessCount <= 0 { - return - } - - // Use partial selection instead of full sort for better performance - // For finding k oldest sessions, we only need O(n) operations - keysToRemove := cm.findOldestSessions(excessCount) - - totalBytesFreed := int64(0) - removedCount := int64(0) - - for _, key := range keysToRemove { - if entry, exists := cm.sessionMap[key]; exists { - totalBytesFreed += entry.SizeEstimate - atomic.AddInt64(&cm.bytesAllocated, -entry.SizeEstimate) - removedCount++ - } - delete(cm.sessionMap, key) - } - - // Update global count - if removedCount > 0 { - atomic.AddInt64(&globalSessionCount, -removedCount) - } - - cm.logger.Infof("Enforced session limit: removed %d excess sessions, freed %d bytes", - len(keysToRemove), totalBytesFreed) -} - -// findOldestSessions returns keys of the k oldest sessions efficiently -// Uses a simple approach: find the kth oldest timestamp, then collect all older entries -func (cm *ChunkManager) findOldestSessions(k int) []string { - if k <= 0 || len(cm.sessionMap) == 0 { - return nil - } - - if k >= len(cm.sessionMap) { - // Remove all sessions - keys := make([]string, 0, len(cm.sessionMap)) - for key := range cm.sessionMap { - keys = append(keys, key) - } - return keys - } - - // Collect all timestamps with keys - type sessionAge struct { - lastUsed time.Time - key string - } - - sessions := make([]sessionAge, 0, len(cm.sessionMap)) - for key, entry := range cm.sessionMap { - sessions = append(sessions, sessionAge{key: key, lastUsed: entry.LastUsed}) - } - - // Partial sort: get the k smallest elements using selection - // This is O(n*k) which is better than O(n log n) when k << n - for i := 0; i < k; i++ { - minIdx := i - for j := i + 1; j < len(sessions); j++ { - if sessions[j].lastUsed.Before(sessions[minIdx].lastUsed) { - minIdx = j - } - } - if minIdx != i { - sessions[i], sessions[minIdx] = sessions[minIdx], sessions[i] - } - } - - // Return the k oldest - result := make([]string, k) - for i := 0; i < k; i++ { - result[i] = sessions[i].key - } - return result -} - -// CanCreateSession checks if a new session can be created within limits -func (cm *ChunkManager) CanCreateSession() (bool, error) { - cm.mutex.RLock() - currentCount := len(cm.sessionMap) - cm.mutex.RUnlock() - - // Hard limit check - never exceed maxSessions - if currentCount >= cm.maxSessions { - cm.logger.Error("Cannot create session: at maximum limit (%d)", cm.maxSessions) - return false, fmt.Errorf("session storage at maximum capacity (%d sessions)", cm.maxSessions) - } - - // Emergency cleanup at 90% capacity - emergencyThreshold := int(float64(cm.maxSessions) * 0.9) - if currentCount >= emergencyThreshold { - cm.logger.Info("Session storage at %d%% capacity, triggering emergency cleanup", - (currentCount*100)/cm.maxSessions) - cm.EmergencyCleanup() - - // Recheck after cleanup - cm.mutex.RLock() - newCount := len(cm.sessionMap) - cm.mutex.RUnlock() - - if newCount >= cm.maxSessions { - return false, fmt.Errorf("session storage full even after emergency cleanup (%d sessions)", newCount) - } - } - - return true, nil -} - -// EmergencyCleanup performs aggressive session cleanup when approaching limits -func (cm *ChunkManager) EmergencyCleanup() { - cm.mutex.Lock() - defer cm.mutex.Unlock() - - now := time.Now() - removed := 0 - - // Remove any expired sessions first - expiredKeys := make([]string, 0) - for key, entry := range cm.sessionMap { - if now.After(entry.ExpiresAt) || now.Sub(entry.LastUsed) > cm.sessionTTL { - expiredKeys = append(expiredKeys, key) - } - } - - for _, key := range expiredKeys { - if entry, exists := cm.sessionMap[key]; exists { - atomic.AddInt64(&cm.bytesAllocated, -entry.SizeEstimate) - } - delete(cm.sessionMap, key) - removed++ - } - - // If still over 80% capacity, remove oldest sessions more aggressively - targetCapacity := int(float64(cm.maxSessions) * 0.8) - if len(cm.sessionMap) > targetCapacity { - excessCount := len(cm.sessionMap) - targetCapacity - - // Use efficient partial sort to find oldest sessions - keysToRemove := cm.findOldestSessions(excessCount) - - for _, key := range keysToRemove { - if entry, exists := cm.sessionMap[key]; exists { - atomic.AddInt64(&cm.bytesAllocated, -entry.SizeEstimate) - } - delete(cm.sessionMap, key) - removed++ - } - } - - cm.lastCleanup = now - cm.logger.Infof("Emergency cleanup completed: removed %d sessions, %d remaining", - removed, len(cm.sessionMap)) - - // Log memory stats after emergency cleanup (read only, no forced GC) - cm.logger.Infof("Sessions after emergency cleanup: %d, Tracked bytes: %d", - len(cm.sessionMap), atomic.LoadInt64(&cm.bytesAllocated)) -} - -// GetSessionCount returns the current number of active sessions (for monitoring) -func (cm *ChunkManager) GetSessionCount() int { - cm.mutex.RLock() - defer cm.mutex.RUnlock() - return len(cm.sessionMap) -} - -// GetMemoryStats returns memory usage statistics for monitoring -func (cm *ChunkManager) GetMemoryStats() map[string]interface{} { - cm.mutex.RLock() - sessionCount := len(cm.sessionMap) - cm.mutex.RUnlock() - - stats := make(map[string]interface{}) - stats["active_sessions"] = sessionCount - stats["max_sessions"] = cm.maxSessions - stats["bytes_allocated"] = atomic.LoadInt64(&cm.bytesAllocated) - stats["peak_sessions"] = atomic.LoadInt64(&cm.peakSessions) - stats["cleanup_count"] = atomic.LoadInt64(&cm.cleanupCount) - stats["session_ttl_hours"] = cm.sessionTTL.Hours() - - // Update peak sessions - if int64(sessionCount) > atomic.LoadInt64(&cm.peakSessions) { - atomic.StoreInt64(&cm.peakSessions, int64(sessionCount)) - } - - return stats }