Skip to content

Fix global profiler values#96048

Merged
antonio2368 merged 1 commit intomasterfrom
fix-global-profiler
Feb 5, 2026
Merged

Fix global profiler values#96048
antonio2368 merged 1 commit intomasterfrom
fix-global-profiler

Conversation

@antonio2368
Copy link
Copy Markdown
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix period for global profilers (controlled by global_profiler_real_time_period_ns and global_profiler_cpu_time_period_ns). Instead of set value, a truncated value was used, causing profiler to wakeup more than intended.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Feb 5, 2026

Workflow [PR], commit [4d63898]

Summary:

job_name test_name status info comment
Finish Workflow failure
python3 ./ci/jobs/scripts/workflow_hooks/new_tests_check.py failure

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Feb 5, 2026
@serxa serxa self-assigned this Feb 5, 2026
@antonio2368 antonio2368 added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Feb 5, 2026
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.

Probably we should also check other places with ?:

@azat
Copy link
Copy Markdown
Member

azat commented Feb 5, 2026

clang-tidy can catch this with --checks=bugprone-narrowing-conversions

test3.cpp
#include <cstddef>
#include <iostream>

struct SettingFieldNumber
{
    using Type = size_t;
    using ValueType = size_t;

    Type value;
    bool changed = false;

    explicit SettingFieldNumber(Type x = 0);
    SettingFieldNumber(const SettingFieldNumber & o)
        : value(o.value), changed(o.changed)
    {}

    virtual SettingFieldNumber & operator=(Type x);
    SettingFieldNumber & operator=(const SettingFieldNumber & o)
    {
        if (this != &o)
        {
            value = o.value;
            changed = o.changed;
        }
        return *this;
    }

    operator Type() const { return value; } /// NOLINT
};

int main()
{
    std::cout << "SettingFieldNumber:: size: " << sizeof(getenv("foo") ? SettingFieldNumber() : 0) << "\n";
    std::cout << "SettingFieldNumber:: size: " << sizeof(getenv("foo") ? SettingFieldNumber().value : 0) << "\n";
    return 0;
}
$ clang-tidy --checks=bugprone-narrowing-conversions /tmp/test3.cpp
Error while trying to load a compilation database:
Could not auto-detect compilation database for file "/tmp/test3.cpp"
No compilation database found in /tmp or any parent directory
fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory
Running without flags.
1 warning generated.
/tmp/test3.cpp:33:74: warning: narrowing conversion from 'Type' (aka 'unsigned long') to signed type 'int' is implementation-defined [bugprone-narrowing-conversions]
   33 |     std::cout << "SettingFieldNumber:: size: " << sizeof(getenv("foo") ? SettingFieldNumber() : 0) << "\n";
      |                                                                          ^

But likely enabling it will have too much of this warnings...

@antonio2368
Copy link
Copy Markdown
Member Author

@azat Let's try and see in a different PR

@antonio2368 antonio2368 added this pull request to the merge queue Feb 5, 2026
Merged via the queue into master with commit ef0461c Feb 5, 2026
258 of 262 checks passed
@antonio2368 antonio2368 deleted the fix-global-profiler branch February 5, 2026 15:43
@robot-clickhouse robot-clickhouse added the pr-synced-to-cloud The PR is synced to the cloud repo label Feb 5, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Feb 5, 2026
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Feb 5, 2026
antonio2368 added a commit that referenced this pull request Feb 6, 2026
Backport #96048 to 25.3: Fix global profiler values
antonio2368 added a commit that referenced this pull request Feb 6, 2026
Backport #96048 to 25.8: Fix global profiler values
antonio2368 added a commit that referenced this pull request Feb 6, 2026
Backport #96048 to 25.11: Fix global profiler values
antonio2368 added a commit that referenced this pull request Feb 6, 2026
Backport #96048 to 25.12: Fix global profiler values
antonio2368 added a commit that referenced this pull request Feb 6, 2026
Backport #96048 to 26.1: Fix global profiler values
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants