tail: reduce CPU load for polling #3618
Conversation
* unifiy delays: 500 ms for inotify, 1500 ms for polling * fix test_follow_name_move2
* unifiy delays: 500 ms for inotify, 1500 ms for polling * fix test_follow_name_move2
This reduces the CPU load for polling drastically (from ~80% down to ~5%) by removing/fixing several previous workarounds related to polling, while still passing all related GNU test-suite checks. * set Notify::PollWatcher delay to: sleep_sec/10 instead of sleep_sec/100 * set recv_timeout to sleep_sec instead of sleep_sec/100 * remove the manual polling of watched files Bugs: * fix an issue with headers to consistently pass "test_follow_name_retry_headers" and "gnu/tests/tail-2/overlay-headers.sh" Code clean-up and refactor * make fields of struct FileHandling private (and add getters/setters) to ensure that the paths are absolute and match the paths returned by Notify::Events * replace calls to "crash!" with "return USimpleError" * clean-up formatting
This reduces the CPU load for polling drastically (from ~80% down to ~5%) by removing/fixing several previous workarounds related to polling, while still passing all related GNU test-suite checks. * set Notify::PollWatcher delay to: sleep_sec/10 instead of sleep_sec/100 * set recv_timeout to sleep_sec instead of sleep_sec/100 Bugs: * fix an issue with headers to consistently pass "test_follow_name_retry_headers" and "gnu/tests/tail-2/overlay-headers.sh" Code clean-up and refactor * make fields of struct FileHandling private (and add getters/setters) to ensure that the paths are absolute and match the paths returned by Notify::Events * replace calls to "crash!" with "return USimpleError" * clean-up formatting
|
I guess you saw: |
* add/fix manual polling to pass:
"gnu/tests/tail-2/F-vs-rename.sh"
"gnu/tests/tail-2/truncate.sh"
* pin notify version because it is a pre-release
* code clean-up & refactoring
Setting notify::PollWatcher::compare_contents to true makes many workarounds for polling unneccessary.
Setting notify::PollWatcher::compare_contents to true makes many workarounds for polling unneccessary.
The overlay-headers.sh test intends to check `tail` for: "redundant headers for overlapping inotify events while it was suspended". However, it then runs `tail ---dis` which disables inotify events, i.e. it only uses polling. Since this test suspends/resumes `tail` and our polling implementation differs slightly from GNU's `tail`, this test is fluctuating between PASS/FAIL, depending on scheduling. There are two issues here: 1. This test does not work as intended because it does not test inotify events. 2. For `---dis` uu_tail passes this test only sometimes. For 1. I opened a bug#55996 at bug-coreutils@gnu.org For 2. this commit uses the same fix as suggested in the bug report by removing the `---dis` flag. With this change the test uses inotify events and in turn `uu_tail` should always pass overlay-headers.sh test.
* remove read_some condition from ProcessChecker::is_dead check to make make pid.sh test pass consistently * remove manual polling (again)
Warning: Congrats! The gnu test tests/tail-2/F-vs-rename is no longer failing!
Warning: Congrats! The gnu test tests/tail-2/overlay-headers is no longer failing!
Error: GNU test failed: tests/tail-2/pid. tests/tail-2/pid is passing on 'main'. Maybe you have to rebase?I run the Could a volunteer please check this branch out and give me feedback if this test is passing/failing on their system? |
Thanks. I think I found the culprit now and the last 5 CI test runs were green. |
|
Sounds good. |
|
The windows tasks seem to be stalled. Do you see that too? |
Yes, I see Windows and macOS still in progress. |
|
They will timeout, i think there is a bug. |
Following the parent dir instead of the file itself is a workaround only intended for linux (inotify).
|
Are you aware of |
|
Finally everything is green.
Yes. I removed the If other tests that use this option fail some times, I would try the same workaround. |
|
I am working on a grcov workaround |
This should fix: #3616, fix: #3633
together with #3606 (comment) and #3611 (comment)
This uses the recently introduced
compare_contentsfeature of notify PollWatcher.This means that there is no more need for extra tight poll intervals and workarounds to catch every event. This also means that the relevant GNU test-suite checks should be more stable now (no more fluctuation between PASS/FAIL).