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
[Enhancement] Add datacache memory tracker to trace the datacache memory usage. #38884
Conversation
mem_metrics->process_mem_bytes.value(), mem_metrics->query_mem_bytes.value(), | ||
mem_metrics->load_mem_bytes.value(), mem_metrics->metadata_mem_bytes.value(), | ||
mem_metrics->compaction_mem_bytes.value(), mem_metrics->schema_change_mem_bytes.value(), | ||
mem_metrics->column_pool_mem_bytes.value(), mem_metrics->storage_page_cache_mem_bytes.value(), | ||
mem_metrics->update_mem_bytes.value(), mem_metrics->chunk_allocator_mem_bytes.value(), | ||
mem_metrics->clone_mem_bytes.value(), mem_metrics->consistency_mem_bytes.value()); | ||
mem_metrics->clone_mem_bytes.value(), mem_metrics->consistency_mem_bytes.value(), | ||
datacache_mem_bytes); | ||
|
||
nap_sleep(15, [daemon] { return daemon->stopped(); }); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most risky bug in this code is:
Potential null pointer dereference.
You can modify the code like this:
// ...
BlockCache* block_cache = BlockCache::instance();
if (block_cache && block_cache->is_initialized()) { // Check if block_cache is not nullptr before using it
auto datacache_metrics = block_cache->cache_metrics();
datacache_mem_bytes = datacache_metrics.mem_used_bytes + datacache_metrics.meta_used_bytes;
}
// ...
Explanation:
In the code provided, there's a check for block_cache->is_initialized()
but no check to ensure that block_cache
is not a nullptr
before calling is_initialized()
on it. If BlockCache::instance()
returns a nullptr
, attempting to call is_initialized()
or any other method on it will lead to undefined behavior and possibly crash the program. Therefore, it's important to add a null pointer check before using block_cache
.
… usage. Signed-off-by: Gavin <yangguansuo@starrocks.com>
ed178c7
to
b695950
Compare
[FE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]❌ fail : 2 / 14 (14.29%) file detail
|
@Mergifyio backport branch-3.2 |
@Mergifyio backport branch-3.1 |
✅ Backports have been created
|
✅ Backports have been created
|
…ory usage. (StarRocks#38884) Signed-off-by: Gavin <yangguansuo@starrocks.com>
…ory usage. (StarRocks#38884) Signed-off-by: Gavin <yangguansuo@starrocks.com>
…sage. (backport StarRocks#38884) Signed-off-by: Gavin <yangguansuo@starrocks.com>
…sage. (backport StarRocks#38884) Signed-off-by: Gavin <yangguansuo@starrocks.com>
…ory usage. (StarRocks#38884) Signed-off-by: Gavin <yangguansuo@starrocks.com> Signed-off-by: Seaven <seaven_7@qq.com>
Why I'm doing:
The datacache memory usage is unobservable, which brings difficulties to analyze process memory details.
What I'm doing:
Add datacache memory tracker to trace the datacache memory usage.
You can observe the datacache memory usage by follow urls:
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: