Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manage watch applied to mulitple services #12469

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Jan 15, 2025

What I did
watch used to be ran per service which brings race condition when a file event has impacts on multiple services.
This PR changes the implementation so we collect file events, then check all services for watch rules matching update, and eventually apply required actions.

Updated logic is :

  • create a single watcher for the paths declared by all services in project
  • debounce raw filesystem event for batch processing
  • select watch rule(s) to match updated paths; apply ignore filter by rule (as distinct service may declare distinct ignores)
  • apply changes to services and paths impacted

Related issue
closes #12427

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@ndeloof ndeloof force-pushed the multi-service-watch branch 7 times, most recently from 45f3ec1 to c95b804 Compare January 16, 2025 13:20
@ndeloof ndeloof requested review from glours and a team January 16, 2025 14:12
@ndeloof ndeloof marked this pull request as ready for review January 16, 2025 14:42
@ndeloof ndeloof force-pushed the multi-service-watch branch 2 times, most recently from 8dced24 to 063889a Compare January 21, 2025 17:17
@ndeloof ndeloof force-pushed the multi-service-watch branch from 063889a to 571c6ec Compare January 21, 2025 17:27
Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me, I didn't detect any regression or behavior changes during my tests 👍
I really like those improvements, thanks @ndeloof 🙏

@glours glours merged commit d04b3f4 into docker:main Jan 22, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Docker Compose watch fails to replace new containers if change affects multiple containers
2 participants