gorakhargosh / watchdog

Python library and shell utilities to monitor filesystem events.
http://packages.python.org/watchdog/
Apache License 2.0
6.48k stars 694 forks source link

Ambiguity in setting FileClosedEvent in filter #1046

Closed xieyuguang closed 3 weeks ago

xieyuguang commented 1 month ago

If the filter is set to FileClosedEvent, the mask is set to InotifyConstants.IN_CLOSE which includes IN_CLOSE_WRITE and IN_CLOSE_NOWRITE, but only IN_CLOSE_WRITE will call on_closed(). I think line 236 should be modified to event_mask |= InotifyConstants.IN_CLOSE_WRITE to improve performance and maintain logical consistency in the code.

https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify.py#L235-L236

https://github.com/gorakhargosh/watchdog/blob/9f23b599f369cdd873c0cf3dd9e69567234d78fb/src/watchdog/observers/inotify.py#L190-L199

BoboTiG commented 1 month ago

I like your proposal! Would you mind opening a PR?

BoboTiG commented 1 month ago

Hm after reading #747, I think we will loose important events. Example: I open a file and get the related event; then if we apply your proposal, I will never know when the file is closed in the case of a non-write. 🤔

xieyuguang commented 1 month ago

It seems that there is currently no way to get the non-write closed event, as the relevant code has been commented out, as shown in lines 197 to 199. Can we more finely distinguish which type of file closed event it is, so that it can be set on the filter and corresponding callbacks can be added for calling, such as on_closed_write() and on_closed_nowrite().

xieyuguang commented 1 month ago

We discovered this issue by having a file upload service that uses this library to detect all newly added files in a certain directory. When these newly added files are written and closed, the upload will be triggered. But we found abnormal CPU usage, which turned out to be another service that scans text content and continuously analyzes the content of text files in the same directory read-only. But our file upload service actually doesn't care about these read-only file closed events.

BoboTiG commented 1 month ago

Can we more finely distinguish which type of file closed event it is, so that it can be set on the filter and corresponding callbacks can be added for calling, such as on_closed_write() and on_closed_nowrite().

It seems a potential solution to me, yes 👍🏻