diff --git a/cmd/main.go b/cmd/main.go index f36c0fbb..9eca4067 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -200,7 +200,14 @@ func main() { // Create store layer that wraps cache with prefixed keys and encapsulated operations dataStore := store.New(cache) - if err = preloadCache(*appConf, dataStore, sharedCacheMutex); err != nil { + // Set up the shared shutdown context up front so the Snowflake async + // continuation (#296) can derive a cancelable child from it and stop + // cleanly when the manager receives SIGINT/SIGTERM. `SetupSignalHandler` + // can only be called once per process; reuse the same context for + // `mgr.Start` below. + shutdownCtx := ctrl.SetupSignalHandler() + + if err = preloadCache(shutdownCtx, *appConf, dataStore, sharedCacheMutex); err != nil { setupLog.Error(err, "failed to preload cache") os.Exit(1) } @@ -260,7 +267,7 @@ func main() { } }() setupLog.Info("starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(shutdownCtx); err != nil { setupLog.Error(err, "problem running manager") os.Exit(1) } @@ -294,11 +301,17 @@ type snowflakeAsyncState struct { // This is done only once at the start of the application // and the cache is flushed when the application is restarted // Optimized to use goroutines for parallel processing of backends -func preloadCache(appConfig config.AppConfig, dataStore *store.Store, cacheMutex *sync.RWMutex) error { - ctx := context.Background() - - // Add request ID for tracking this cache preload operation in logs - ctx = logger.WithRequestId(ctx, types.UID(uuid.New().String())) +func preloadCache( + shutdownCtx context.Context, + appConfig config.AppConfig, + dataStore *store.Store, + cacheMutex *sync.RWMutex, +) error { + // Derive from the shutdown context so any background continuation kicked + // off after preload (e.g. Snowflake async, #296) gets canceled on + // shutdown. preloadCache itself is synchronous so this only matters for + // long-lived children launched below. + ctx := logger.WithRequestId(shutdownCtx, types.UID(uuid.New().String())) // Use errgroup for concurrent processing with proper error handling g, ctx := errgroup.WithContext(ctx) @@ -419,10 +432,13 @@ func preloadCache(appConfig config.AppConfig, dataStore *store.Store, cacheMutex setupLog.Info("cache preload completed for all backends") - // Start async continuation for all Snowflake backends (after all preloads done) + // Start async continuation for all Snowflake backends (after all preloads done). + // Pass the shutdownCtx (not the errgroup ctx, which gets canceled when g.Wait + // returns) so the goroutine survives preloadCache returning yet still gets + // canceled when the manager receives SIGINT/SIGTERM (#296). for _, state := range snowflakeStates { if state.lastUser != "" { - startSnowflakeAsyncContinuation(ctx, state, dataStore, cacheMutex) + startSnowflakeAsyncContinuation(shutdownCtx, state, dataStore, cacheMutex) } } @@ -431,17 +447,24 @@ func preloadCache(appConfig config.AppConfig, dataStore *store.Store, cacheMutex // startSnowflakeAsyncContinuation starts a background goroutine to fetch remaining Snowflake users // and write them to cache. This function returns immediately after starting the goroutine. +// +// shutdownCtx must be the long-lived manager shutdown context (typically +// `ctrl.SetupSignalHandler()`'s result) so the goroutine gets canceled when +// the manager receives SIGINT/SIGTERM. Passing `context.Background()` here +// leaks the goroutine across shutdown — see #296. func startSnowflakeAsyncContinuation( - originalCtx context.Context, + shutdownCtx context.Context, state *snowflakeAsyncState, dataStore *store.Store, cacheMutex *sync.RWMutex, ) { - // Create a fresh context since the errgroup context is canceled after g.Wait() returns - // Transfer the logger (with request ID) from original context for traceability - asyncCtx := context.Background() - if entry := logger.Logger(originalCtx); entry != nil { - asyncCtx = context.WithValue(asyncCtx, logger.RequestIdKey, entry) + // Derive a child context from the manager's shutdown context so SIGINT/SIGTERM + // propagates into FetchRemainingUsersAsync and the cache writes below. + // Transfer the request-id logger value from the originating context for + // traceability. + asyncCtx := shutdownCtx + if entry := logger.Logger(shutdownCtx); entry != nil { + asyncCtx = context.WithValue(shutdownCtx, logger.RequestIdKey, entry) } userChan, errChan := state.client.FetchRemainingUsersAsync(asyncCtx, state.lastUser)