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.

Sign up for GitHub

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

Jump to bottom

Fix to main loop timeout calculation leading to a tight loop for a max period of 1 ms #4671

Merged
merged 1 commit into from
Apr 5, 2024
Merged

Fix to main loop timeout calculation leading to a tight loop for a max period of 1 ms #4671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ librdkafka v2.3.1 is a maintenance release:
* Integration tests can be started in KRaft mode and run against any
GitHub Kafka branch other than the released versions.
* Fix pipeline inclusion of static binaries (#4666)
* Fix to main loop timeout calculation leading to a tight loop for a
max period of 1 ms (#4671).
Comment on lines +10 to +11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use something easier to understand than tight loop. The later statement is more comprehendible than this.



## Fixes
Expand All @@ -20,6 +22,10 @@ librdkafka v2.3.1 is a maintenance release:
Solved by correctly excluding the binary configured with that library,
when targeting a static build.
Happening since v2.0.2, with specified platforms, when using static binaries (#4666).
* When the main thread loop was awakened less than 1 ms
before the expiration of a timeout, it was serving with a zero timeout,
leading to increased CPU usage until the timeout was reached.
Happening since 1.x (#4671).



Expand Down
5 changes: 4 additions & 1 deletion src/rdkafka.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2120,7 +2120,10 @@ static int rd_kafka_thread_main(void *arg) {
RD_KAFKA_CGRP_STATE_TERM)))) {
rd_ts_t sleeptime = rd_kafka_timers_next(
&rk->rk_timers, 1000 * 1000 /*1s*/, 1 /*lock*/);
rd_kafka_q_serve(rk->rk_ops, (int)(sleeptime / 1000), 0,
/* Use ceiling division to avoid calling serve with a 0 ms
* timeout in a tight loop until 1 ms has passed. */
int timeout_ms = (sleeptime + 999) / 1000;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had one concern with the overflow but that will never happen as we use CLOCK_MONOTONIC and hence will never reach the limit for rd_ts_t.

rd_kafka_q_serve(rk->rk_ops, timeout_ms, 0,
RD_KAFKA_Q_CB_CALLBACK, NULL, NULL);
if (rk->rk_cgrp) /* FIXME: move to timer-triggered */
rd_kafka_cgrp_serve(rk->rk_cgrp);
Expand Down