Skip to content

Reduce logging level for Throttler (too noisy)#101035

Merged
azat merged 1 commit intoClickHouse:masterfrom
azat:throttler-logging
Mar 29, 2026
Merged

Reduce logging level for Throttler (too noisy)#101035
azat merged 1 commit intoClickHouse:masterfrom
azat:throttler-logging

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented Mar 28, 2026

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

@azat azat requested a review from serxa March 28, 2026 20:19
@azat azat mentioned this pull request Mar 28, 2026
Merged
1 task
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 28, 2026 •
edited
Loading

Workflow [PR], commit [30ab84b]

Summary: ❌

job_name test_name status info comment
Stress test (amd_debug) failure
Cannot start clickhouse-server FAIL cidb
Parse failure error failure cidb
Check failed failure cidb

AI Review

Summary

This PR reduces verbosity in Throttler::throttle by changing the "Not sleeping" log from LOG_TRACE to LOG_TEST in src/Common/Throttler.cpp. I did not find correctness, safety, performance, or compatibility issues in the code change. The selected Changelog category (Not for changelog) matches the non-user-facing nature of the change.

ClickHouse Rules
Item Status Notes
Deletion logging ➖
Serialization versioning ➖
Core-area scrutiny ✅
No test removal ✅
Experimental gate ➖
No magic constants ✅
Backward compatibility ✅
SettingsChangesHistory.cpp ➖
PR metadata quality ✅
Safe rollout ✅
Compilation time ✅
Final Verdict
  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 28, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Mar 28, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.10% 84.10% +0.00%
Functions 24.50% 24.50% +0.00%
Branches 76.70% 76.70% +0.00%

Changed lines: 100.00% (6/6) · Uncovered code

Full report · Diff report

serxa
serxa approved these changes Mar 29, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@serxa serxa left a comment

Choose a reason for hiding this comment

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

TBH intention for the logging was to find the root cause of some flaky test and to be able to diagnose better throttling related code. Maybe we should even introduce a query parameter to enable the logging. Anyway, probably this change should resolve current issue. Thanks

@azat azat enabled auto-merge March 29, 2026 13:15
@azat
Copy link
Copy Markdown
Member Author

azat commented Mar 29, 2026

Stress test (amd_debug)

@azat azat added this pull request to the merge queue Mar 29, 2026
Merged via the queue into ClickHouse:master with commit cef81be Mar 29, 2026
151 of 153 checks passed
@azat azat deleted the throttler-logging branch March 29, 2026 13:36
@azat
Copy link
Copy Markdown
Member Author

azat commented Mar 29, 2026

BTW I'm experimenting with perfetto support in chdig, and this is how I found it

image

As you can see tons of such messages, and also our ci-logs cluster sometimes looses logs because over the queue overflow, likely this is the culprit

@robot-ch-test-poll4 robot-ch-test-poll4 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 29, 2026
Desel72 pushed a commit to Desel72/ClickHouse that referenced this pull request Mar 30, 2026
@azat
Merge pull request ClickHouse#101035 from azat/throttler-logging
1fb8796 
Reduce logging level for Throttler (too noisy)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serxa serxa serxa approved these changes

Assignees

No one assigned

Labels

pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@azat @serxa @robot-ch-test-poll4