-
-
Notifications
You must be signed in to change notification settings - Fork 7k
ratelimit: avoid tight loop when transfer is paused #20091
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
ratelimit: avoid tight loop when transfer is paused #20091
Conversation
When a transfer is paused via curl_easy_pause(), the progress callback gets invoked excessively (hundreds of thousands of times per second), causing 100% CPU usage. The issue occurs because Curl_rlimit_wait_ms() returns 0 when blocked, preventing the multi-handle from entering MSTATE_RATELIMITING state. This causes the event loop to spin without any delay. When blocked (paused), return 1000ms wait time to ensure the multi-handle properly waits between iterations while still allowing periodic progress callback updates for UI responsiveness. Reproduced with: ~260,000 callbacks/sec, 100% CPU usage After fix: ~4-5 callbacks/sec, 0.2% CPU usage Follow-up to 24b36fd
|
Analysis of PR #20091 at bcc135f6: Test ../../tests/http/test_07_upload.py::TestUpload::test_07_42c_upload_disconnect[h2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_42a_upload_disconnect[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Test ../../tests/http/test_07_upload.py::TestUpload::test_07_42a_upload_disconnect[h2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Test ../../tests/http/test_07_upload.py::TestUpload::test_07_42b_upload_disconnect[h2] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Generated by Testclutch |
|
Clearly, this breaks tests. |
|
@icing do you have any alternative take on this? |
Fix the pollset in perform state to not add sockets for directions that are blocked. This otherwise will lead to busy loops for a transfer that cannot be progressed. refs curl#20091
|
@Fizn-Ahmd thanks for your report and finding that problem. The bug is that the transfer socket is still being monitored for events, even though the send/recv is blocked and never handles the sockets. This raised the socket event again and this continues over and over. While your proposed fix mediates this problem in your case by an imposed idle wait, I propose an alternate fix in #20109, where sockets are no longer monitored for a blocked direction. It would be great if you could verify that this works for you as well. Thanks! |
When a transfer is paused via curl_easy_pause(), the progress callback gets invoked excessively (hundreds of thousands of times per second), causing 100% CPU usage.
The issue occurs because Curl_rlimit_wait_ms() returns 0 when blocked, preventing the multi-handle from entering MSTATE_RATELIMITING state. This causes the event loop to spin without any delay.
When blocked (paused), return 1000ms wait time to ensure the multi-handle properly waits between iterations while still allowing periodic progress callback updates.
Reproduced with: ~260,000 callbacks/sec, 100% CPU usage
After fix: ~4-5 callbacks/sec, 0.2% CPU usage
Follow-up to 24b36fd