Commit 50e9dba

Eric Bower  ·  2026-08-15 14:08:35 -0400 EDT
parent eb0002c
fix(gc): wait 3 hours before killing sessions
2 files changed,  +29, -26
+21, -19
 1@@ -2215,16 +2215,15 @@ func cancelJobSessions(prefix string) {
 2 	}
 3 }
 4 
 5-// findSessionsForGC identifies finished sessions or sessions older than 3 hours matching given prefixes.
 6-// It checks whether a session's job has an active runner session (a .runner session with Ended == "");
 7-// if so, finished child sessions for that job are NOT cleaned up yet to ensure no new tasks spawn
 8-// in pico.sh before cleaning up tasks.
 9+// findSessionsForGC identifies finished or running sessions older than the retention cutoff (3 hours) matching given prefixes.
10+// It checks whether a session's job has an active session; active jobs are preserved unless their sessions have expired.
11+// Finished sessions are kept until they are older than the cutoff so users can inspect/debug test failures.
12 func findSessionsForGC(sessions []SessionInfo, prefixes []string, now time.Time) []string {
13 	cutoff := now.Add(-3 * time.Hour).Unix()
14 
15 	// Track which job prefixes have ANY active (running) session.
16 	// As long as any session for a job is still running (Ended == ""),
17-	// the job is active and none of its sessions should be garbage collected.
18+	// the job is active and none of its sessions should be garbage collected unless expired.
19 	activeJobPrefixes := make(map[string]bool)
20 	for _, s := range sessions {
21 		if s.Ended == "" {
22@@ -2248,7 +2247,7 @@ func findSessionsForGC(sessions []SessionInfo, prefixes []string, now time.Time)
23 			continue
24 		}
25 
26-		// If the session belongs to a job with an active runner, do not kill it
27+		// If the session belongs to a job with an active session, do not kill it
28 		// unless it has expired (> 3 hours old).
29 		jobPrefix := extractJobPrefix(s.Name)
30 		if jobPrefix != "" && activeJobPrefixes[jobPrefix] {
31@@ -2264,22 +2263,25 @@ func findSessionsForGC(sessions []SessionInfo, prefixes []string, now time.Time)
32 			continue
33 		}
34 
35+		// For completed/inactive jobs, only kill sessions if they ended or were created before the cutoff
36 		if s.Ended != "" {
37-			toKill = append(toKill, s.Name)
38-			continue
39-		}
40-
41-		if s.Created == "" {
42-			continue
43-		}
44-
45-		var created int64
46-		if _, err := fmt.Sscanf(s.Created, "%d", &created); err != nil {
47-			continue
48+			var ended int64
49+			if _, err := fmt.Sscanf(s.Ended, "%d", &ended); err == nil {
50+				if ended < cutoff {
51+					toKill = append(toKill, s.Name)
52+				}
53+				continue
54+			}
55 		}
56 
57-		if created < cutoff {
58-			toKill = append(toKill, s.Name)
59+		if s.Created != "" {
60+			var created int64
61+			if _, err := fmt.Sscanf(s.Created, "%d", &created); err != nil {
62+				continue
63+			}
64+			if created < cutoff {
65+				toKill = append(toKill, s.Name)
66+			}
67 		}
68 	}
69 	return toKill
+8, -7
 1@@ -706,9 +706,11 @@ func TestFindSessionsForGC(t *testing.T) {
 2 
 3 	sessions := []SessionInfo{
 4 		{Name: "ci.repo.active.runner", Created: twoHoursAgo, Ended: ""},
 5-		{Name: "ci.repo.active.step.fmt", Created: twoHoursAgo, Ended: "1786289500"},
 6-		{Name: "ci.repo.finished.runner", Created: twoHoursAgo, Ended: "1786289600"},
 7-		{Name: "ci.repo.finished.step.lint", Created: twoHoursAgo, Ended: "1786289550"},
 8+		{Name: "ci.repo.active.step.fmt", Created: twoHoursAgo, Ended: twoHoursAgo},
 9+		{Name: "ci.repo.recent.runner", Created: twoHoursAgo, Ended: twoHoursAgo},
10+		{Name: "ci.repo.recent.step.lint", Created: twoHoursAgo, Ended: twoHoursAgo},
11+		{Name: "ci.repo.oldfinished.runner", Created: fourHoursAgo, Ended: fourHoursAgo},
12+		{Name: "ci.repo.oldfinished.step.lint", Created: fourHoursAgo, Ended: fourHoursAgo},
13 		{Name: "ci.repo.expired.runner", Created: fourHoursAgo, Ended: ""},
14 		{Name: "other.session", Created: fourHoursAgo, Ended: ""},
15 	}
16@@ -718,9 +720,9 @@ func TestFindSessionsForGC(t *testing.T) {
17 		t.Fatalf("expected 3 sessions for GC, got %d: %v", len(toKill), toKill)
18 	}
19 	expected := map[string]bool{
20-		"ci.repo.finished.runner":    true,
21-		"ci.repo.finished.step.lint": true,
22-		"ci.repo.expired.runner":     true,
23+		"ci.repo.oldfinished.runner":    true,
24+		"ci.repo.oldfinished.step.lint": true,
25+		"ci.repo.expired.runner":        true,
26 	}
27 	for _, k := range toKill {
28 		if !expected[k] {
29@@ -1495,4 +1497,3 @@ func TestFindSSHAuthMethod_ExplicitKeyAndCert(t *testing.T) {
30 		t.Fatal("expected error when non-existent certificate is explicitly specified")
31 	}
32 }
33-