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

watch: batch & de-duplicate file events #10865

Merged
merged 1 commit into from Aug 3, 2023
Merged

Conversation

milas
Copy link
Member

@milas milas commented Aug 2, 2023

Adjust the debouncing logic so that it applies to all inbound file events, regardless of whether they match a sync or rebuild rule.

When the batch is flushed out, if any event for the service is a rebuild event, then the service is rebuilt and all sync events for the batch are ignored. If all events in the batch are sync events, then a sync is triggered, passing the entire batch at once. This provides a substantial performance win for the new tar-based implementation, as it can efficiently transfer the changes in bulk.

Additionally, this helps with jitter, e.g. it's not uncommon for there to be double-writes in quick succession to a file, so even if there's not many files being modified at once, it can still prevent some unnecessary transfers.

What I did

Related issue
JIRA: https://docker.atlassian.net/browse/ENV-191

(not mandatory) A picture of a cute animal, if possible in relation to what you did
a line of dogs holding their food bowls in their mouths waiting to be fed

Adjust the debouncing logic so that it applies to all inbound file
events, regardless of whether they match a sync or rebuild rule.

When the batch is flushed out, if any event for the service is a
rebuild event, then the service is rebuilt and all sync events for
the batch are ignored. If _all_ events in the batch are sync events,
then a sync is triggered, passing the entire batch at once. This
provides a substantial performance win for the new `tar`-based
implementation, as it can efficiently transfer the changes in bulk.

Additionally, this helps with jitter, e.g. it's not uncommon for
there to be double-writes in quick succession to a file, so even if
there's not many files being modified at once, it can still prevent
some unnecessary transfers.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
@milas milas requested a review from a team August 2, 2023 14:07
@milas milas self-assigned this Aug 2, 2023
@milas milas requested review from nicksieger, ndeloof, StefanScherer, ulyssessouza, glours and laurazard and removed request for a team August 2, 2023 14:07
@codecov
Copy link

codecov bot commented Aug 2, 2023

Codecov Report

Patch coverage: 25.17% and project coverage change: -1.10% ⚠️

Comparison is base (8318f66) 59.88% compared to head (1471106) 58.79%.
Report is 8 commits behind head on v2.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10865      +/-   ##
==========================================
- Coverage   59.88%   58.79%   -1.10%     
==========================================
  Files         118      119       +1     
  Lines       10064    10368     +304     
==========================================
+ Hits         6027     6096      +69     
- Misses       3440     3673     +233     
- Partials      597      599       +2     
Files Changed Coverage Δ
internal/sync/docker_cp.go 0.00% <0.00%> (ø)
internal/sync/tar.go 0.00% <0.00%> (ø)
pkg/compose/watch.go 37.76% <47.78%> (+11.12%) ⬆️
pkg/compose/compose.go 61.32% <100.00%> (+0.21%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@milas milas merged commit 3b0742f into docker:v2 Aug 3, 2023
25 checks passed
@milas milas deleted the watch-debounce-batch branch August 3, 2023 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants