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.

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

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

Conversation

emasab
Copy link
Contributor

@emasab emasab commented Apr 3, 2024

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

@emasab emasab force-pushed the dev_fix_main_thread_tight_loop branch from 1315712 to 37b4ef4 Compare April 3, 2024 13:28
@emasab emasab force-pushed the dev_metadata_cache_topic_id_and_fixes branch from ba3ee8d to f405243 Compare April 3, 2024 13:39
@emasab emasab force-pushed the dev_fix_main_thread_tight_loop branch from 37b4ef4 to f5ffbf8 Compare April 3, 2024 13:40
@emasab emasab force-pushed the dev_metadata_cache_topic_id_and_fixes branch from f405243 to 4dd41b7 Compare April 3, 2024 14:26
@emasab emasab force-pushed the dev_fix_main_thread_tight_loop branch from f5ffbf8 to 7403c10 Compare April 3, 2024 16:53
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.

Comment on lines +14 to +11
* Fix to main loop timeout calculation leading to a tight loop for a
max period of 1 ms (#4671).
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.

pranavrth
pranavrth previously approved these changes Apr 4, 2024
@emasab emasab changed the base branch from dev_metadata_cache_topic_id_and_fixes to master April 5, 2024 07:59
@emasab emasab dismissed pranavrth’s stale review April 5, 2024 07:59

The base branch was changed.

leading to a tight loop for a max period of 1 ms

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
@emasab emasab force-pushed the dev_fix_main_thread_tight_loop branch from 2b56329 to 6a8f931 Compare April 5, 2024 08:10
@emasab emasab merged commit 807b23a into master Apr 5, 2024
2 checks passed
@emasab emasab deleted the dev_fix_main_thread_tight_loop branch April 5, 2024 18:37
anchitj pushed a commit that referenced this pull request Jun 10, 2024
leading to a tight loop for a max period of 1 ms

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants