Select polling based watcher for docker log file watcher on Windows #37412
Conversation
daemon/logger/loggerutils/logfile.go
Outdated
| // Becuase of the OS lazy writing, we don't get notifications for file writes and thereby the watcher | ||
| // doesn't work. Hence for Windows we will use poll based notifier. | ||
| if runtime.GOOS == "windows" { | ||
| return watchFileForWindows(name) |
There was a problem hiding this comment.
This ends up duplicating a lot.
Can we set fileWatcher to a polling watcher for Windows and keep using the rest of this function?
There was a problem hiding this comment.
Thank you @cpuguy83 I updated the method as per your comments.
Codecov Report
@@ Coverage Diff @@
## master #37412 +/- ##
=========================================
Coverage ? 34.93%
=========================================
Files ? 610
Lines ? 44875
Branches ? 0
=========================================
Hits ? 15678
Misses ? 27078
Partials ? 2119 |
mnltejaswini
force-pushed
the
naduggar/logissue
branch
from
1f479a4
to
0de6721
Compare
daemon/logger/loggerutils/logfile.go
Outdated
| return nil, err | ||
| } | ||
|
|
||
| if err := fileWatcher.Add(name); err == nil { |
There was a problem hiding this comment.
Can probably keep this check outside the if-block. Not particularly harmful to error out twice in the case of Windows.
There was a problem hiding this comment.
Not sure I followed this comment. For non Windows we retry with polling right?
There was a problem hiding this comment.
Right. So some pseudo-code if windows { fileWatcher = getPoller } else { fileWatcher = getGeneric}; fileWatcher.Add(...) || fileWatcher = getPoller; fileWatcer.Add(...) || exit
There was a problem hiding this comment.
Wouldn't the fallback logic be duplicate for Windows?
There was a problem hiding this comment.
Yes, but doesn't really hurt anything.
There was a problem hiding this comment.
Ok. I updated the code as per your comment.
Signed-off-by: Tejaswini Duggaraju <naduggar@microsoft.com>
mnltejaswini
force-pushed
the
naduggar/logissue
branch
from
0de6721
to
df84cdd
Compare
|
The test case that failed is My change is not directly related to it. Is it expected or do I need to investigate? |
|
Unrelated testing bug. |
|
LGTM |
Signed-off-by: Tejaswini Duggaraju naduggar@microsoft.com
Fixes Windows docker -f doesn't follow instantly #30046
What I did
I updated logger to use polling based watcher for Windows . This is because the file watcher based on syscall notifications doesn't work because of file caching.
Based on runtime OS, I selected poller based watcher for Windows.
docker run -it --name <containername> <container image>docker logs -f <containername>This should stream the logs of the container continuously.
Check for the runtime OS and use poller based watcher to work around the file caching issue in Windows