Skip to content

Better introspection of in-memory caches#81023

Merged
azat merged 16 commits intoClickHouse:masterfrom
azat:cache-metrics
Jun 3, 2025
Merged

Better introspection of in-memory caches#81023
azat merged 16 commits intoClickHouse:masterfrom
azat:cache-metrics

Conversation

@azat
Copy link
Copy Markdown
Member

@azat azat commented May 30, 2025

Changelog category (leave one):

  • Improvement

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

Better introspection of in-memory caches (expose information about caches in system.metrics over incomplete system.asynchronouse_metrics). Add in-memory caches size (in bytes) into dashboard.html. VectorSimilarityIndexCacheSize/IcebergMetadataFilesCacheSize has been renamed to VectorSimilarityIndexCacheBytes/IcebergMetadataFilesCacheBytes.

Previously most of the caches was exposed via system.asynchronous_metrics, but it is easy to forget to write some extra code in the ServerAsynchronousMetrics.cpp, and there were such places already.

So this patch introduce more general interface for them, adds CurrenMetrics for each cache, via CacheBase and their policies.

Also show summary cache sizes in /dashboard.html

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented May 30, 2025

Workflow [PR], commit [e3b42a0]

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label May 30, 2025
azat added 3 commits May 30, 2025 12:00
Previously most of the caches was exposed via
system.asynchronous_metrics, but it is easy to forget to write some
extra code in the ServerAsynchronousMetrics, and there were such places
already.

So this patch introduce more general interface for them, adds
CurrenMetrics for each cache, via CacheBase and their policies.

Also show summary cache sizes in /dashboard.html

(And also `*Size` had been renamed to `*Bytes`)
@azat azat force-pushed the cache-metrics branch from f0cfb41 to 2971919 Compare May 30, 2025 10:02
@azat
Copy link
Copy Markdown
Member Author

azat commented May 30, 2025

image

@antaljanosbenjamin antaljanosbenjamin self-assigned this May 30, 2025
azat added 5 commits May 30, 2025 16:08
Otherwise this will not allow us to share the same counter, i.e. for
page cache we have one counter that is used for multiple cache shards.
Likely this is the reason of CI failures for 03277_prewarm_cache_2,
where we have 2 more in MarkCacheFiles, since previously it was not
accounted at all in asynchornouse_metrics, while now, since it shares
metric they does

CI: https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=81023&sha=b140973235744f0fea34daa6272cfa4602d72a5b&name_0=PR&name_1=Fast+test&name_2=Tests
        /home/ubuntu/actions-runner/_work/clickhouse-private/clickhouse-private/src/Common/LRUCachePolicy.h:47:9: error: Call to virtual method 'LRUCachePolicy::clear' during destruction bypasses virtual dispatch [clang-analyzer-optin.cplusplus.VirtualCall,-warnings-as-errors]
           47 |         clear();
              |         ^
        /home/ubuntu/actions-runner/_work/clickhouse-private/clickhouse-private/src/Storages/StorageSharedMergeTree.cpp:4020:1: note: Calling '~LRUCachePolicy'
         4020 | }
              | ^
        /home/ubuntu/actions-runner/_work/clickhouse-private/clickhouse-private/src/Common/LRUCachePolicy.h:47:9: note: Call to virtual method 'LRUCachePolicy::clear' during destruction bypasses virtual dispatch
           47 |         clear();
              |         ^~~~~~~
@azat
Copy link
Copy Markdown
Member Author

azat commented May 31, 2025

@antaljanosbenjamin this one should be ready now (private build should OK as well)

azat added a commit to azat/chdig that referenced this pull request May 31, 2025
Note, that this query adopted for all version, it takes into respect all
metrics like VectorSimilarityIndexCacheSize/IcebergMetadataFilesCacheSize, but also
new metrics after ClickHouse/ClickHouse#81023

Fixes: #109
@azat azat requested a review from antaljanosbenjamin June 2, 2025 06:51
Copy link
Copy Markdown
Member

@antaljanosbenjamin antaljanosbenjamin left a comment

Choose a reason for hiding this comment

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

LGTM!

@azat azat enabled auto-merge June 3, 2025 14:47
@azat azat added this pull request to the merge queue Jun 3, 2025
Merged via the queue into ClickHouse:master with commit a7558d3 Jun 3, 2025
116 of 123 checks passed
@azat azat deleted the cache-metrics branch June 3, 2025 15:12
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jun 3, 2025
@azat azat mentioned this pull request Jun 3, 2025
1 task
azat added a commit to azat/ClickHouse that referenced this pull request Jul 14, 2025
azat added a commit to azat/ClickHouse that referenced this pull request Jul 14, 2025
Previous all the following metrics was using metric without `Index` prefix:
- `IndexUncompressedCacheBytes`
- `IndexUncompressedCacheCells`
- `IndexMarkCacheBytes`
- `IndexMarkCacheFiles`

Follow-up for: ClickHouse#81023
azat added a commit to azat/ClickHouse that referenced this pull request Jul 14, 2025
Previous all the following metrics was using metric without `Index` prefix:
- `IndexUncompressedCacheBytes`
- `IndexUncompressedCacheCells`
- `IndexMarkCacheBytes`
- `IndexMarkCacheFiles`

Follow-up for: ClickHouse#81023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-improvement Pull request with some product improvements 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.

6 participants