Repository: containerd/containerd
File: plugins/gc/scheduler.go (Go)

Code before:

			// Check if condition should cause immediate collection.
			if triggered ||
				(s.deletionThreshold > 0 && deletions >= s.deletionThreshold) ||
				(nextCollection == nil && ((s.deletionThreshold == 0 && deletions > 0) ||
					(s.mutationThreshold > 0 && mutations >= s.mutationThreshold))) {
				// Check if not already scheduled before delay threshold
				if nextCollection == nil || nextCollection.After(time.Now().Add(s.scheduleDelay)) {
					// TODO(dmcg): track re-schedules for tuning schedule config
					schedC, nextCollection = schedule(s.scheduleDelay)
				}
			}

			continue
		case <-ctx.Done():
			return
		}

		s.waiterL.Lock()

		stats, err := s.c.GarbageCollect(ctx)
		last := time.Now()
		if err != nil {
			log.G(ctx).WithError(err).Error("garbage collection failed")
			collectionCounter.WithValues("fail").Inc()
			if lastCollection == nil {
				lastCollection = &last
			}
			// Reschedule garbage collection for same duration + 1 second
			schedC, nextCollection = schedule(nextCollection.Sub(*lastCollection) + time.Second)

			// Update last collection time even though failure occurred
			lastCollection = &last

			for _, w := range s.waiters {
				close(w)
			}
			s.waiters = nil
			s.waiterL.Unlock()
			continue
		}

		gcTime := stats.Elapsed()
		gcTimeHist.Update(gcTime)
		log.G(ctx).WithField("d", gcTime).Trace("garbage collected")
		gcTimeSum += gcTime
		collections++
		collectionCounter.WithValues("success").Inc()
		triggered = false
		deletions = 0
		mutations = 0

Review comment (bug):
What do you think about handling the `nil` case more explicitly, similar to what the [original issue reporter suggested](https://github.com/containerd/containerd/issues/12164#issue-3288407129)?
```go
	var retryDelay time.Duration
	if lastCollection != nil {
		// If we have a previous collection time, reschedule based on that interval.
		retryDelay = nextCollection.Sub(*lastCollection) + time.Second
	} else {
		// If this is the first collection and it failed, use the default schedule delay.
		retryDelay = s.scheduleDelay
	}
	schedC, nextCollection = schedule(retryDelay)
```

This explicitly handles the two failure states: failure on the first GC run (`lastCollection == nil`) and failure on a subsequent GC run.  This also removes the repeated assignment to `lastCollection`, which I think makes the error handling path cleaner.

Let me know what you think.

Code after:

			// Check if condition should cause immediate collection.
			if triggered ||
				(s.deletionThreshold > 0 && deletions >= s.deletionThreshold) ||
				(nextCollection == nil && ((s.deletionThreshold == 0 && deletions > 0) ||
					(s.mutationThreshold > 0 && mutations >= s.mutationThreshold))) {
				// Check if not already scheduled before delay threshold
				if nextCollection == nil || nextCollection.After(time.Now().Add(s.scheduleDelay)) {
					// TODO(dmcg): track re-schedules for tuning schedule config
					schedC, nextCollection = schedule(s.scheduleDelay)
				}
			}

			continue
		case <-ctx.Done():
			return
		}

		s.waiterL.Lock()

		stats, err := s.c.GarbageCollect(ctx)
		last := time.Now()
		if err != nil {
			log.G(ctx).WithError(err).Error("garbage collection failed")
			collectionCounter.WithValues("fail").Inc()
			var retryDelay time.Duration
			if lastCollection != nil {
				// If we have a previous collection time, reschedule based on that interval.
				retryDelay = nextCollection.Sub(*lastCollection) + time.Second
			} else {
				// If this is the first collection and it failed, use the default schedule delay.
				retryDelay = s.scheduleDelay
			}
			schedC, nextCollection = schedule(retryDelay)

			// Update last collection time even though failure occurred
			lastCollection = &last

			for _, w := range s.waiters {
				close(w)
			}
			s.waiters = nil
			s.waiterL.Unlock()
			continue
		}

		gcTime := stats.Elapsed()
		gcTimeHist.Update(gcTime)
		log.G(ctx).WithField("d", gcTime).Trace("garbage collected")
		gcTimeSum += gcTime
		collections++