Fix: Robust shutdown and error handling for large file change batches in docker compose watch #13525
Conversation
pkg/compose/watch.go
Outdated
| select { | ||
| case <-ctx.Done(): | ||
| options.LogTo.Log(api.WatchLogger, "Watch disabled") | ||
| options.LogTo.Log(api.WatchLogger, "Watch disabled (context canceled)") |
There was a problem hiding this comment.
"context" doesn't make any sense for end-user
| ctx, cancel := context.WithCancel(ctx) | ||
| defer cancel() | ||
|
|
||
| // debounce and group filesystem events so that we capture IDE saving many files as one "batch" event |
pkg/compose/watch.go
Outdated
| case batch := <-batchEvents: | ||
| case batch, ok := <-batchEvents: | ||
| if !ok { | ||
| options.LogTo.Log(api.WatchLogger, "Watch event channel closed") |
There was a problem hiding this comment.
WatchLogger is used to provide end-user message on the TUI, we should not refer to technical / implementation details
Head branch was pushed to by a user without write access
pkg/compose/create.go
Outdated
| } | ||
|
|
||
| err = s.ensureImagesExists(ctx, project, options.Build, options.QuietPull) | ||
| prepareNetworks(project) |
There was a problem hiding this comment.
i just tried to fix an issue, i just changed reorders the create function to ensure networks and volumes are provisioned before building images, which fixes a test failure
There was a problem hiding this comment.
AFAICT this "fix" is unrelated to the current PR, and I can't see a test failure on main branch. Please avoid such undocumented changes.
There was a problem hiding this comment.
ok, i have reverted back the irrelevant changes, take a look now and lets try running workflows.
pkg/compose/create.go
Outdated
| } | ||
|
|
||
| err = s.ensureImagesExists(ctx, project, options.Build, options.QuietPull) | ||
| prepareNetworks(project) |
There was a problem hiding this comment.
AFAICT this "fix" is unrelated to the current PR, and I can't see a test failure on main branch. Please avoid such undocumented changes.
amyssnippet
force-pushed
the
main
branch
2 times, most recently
from
52025fd to
3f43fdc
Compare
|
You PR is now mixing changes from upstream. Please squash commit and prefer rebase over merge from main |
Ensured all watcher and sync goroutines and channels are robustly closed on context cancellation or error. Added explicit logging for large batches and context cancellation to prevent stuck processes and ensure graceful shutdown on Ctrl-C. Signed-off-by: Amol Yadav <amyssnipet@yahoo.com>
|
@ndeloof now?? |
2 participants
What I did
Related issue
This PR addresses an issue where docker compose up --watch could become unresponsive after a large number of file changes (such as after a branch switch), causing logs to stop and Ctrl-C to not exit cleanly. The issue is #13136 . The fix ensures that all watcher and sync goroutines and channels are robustly closed on context cancellation or error, and adds explicit logging for large batches and context cancellation. This prevents the process from getting stuck and ensures graceful shutdown, even after large file change events.
By fixing: