Fix inaccurate MemoryTracking metric in case of allocation failures#72603
Fix inaccurate MemoryTracking metric in case of allocation failures#72603antaljanosbenjamin merged 1 commit intoClickHouse:masterfrom
Conversation
|
This is an automated comment for commit 88a3dbe with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
|
The changes looks good and it would be nice to have a test to verify. Maybe create a query with restricted memory and create some big array some similar to allocate big chunk of memory? I guess we can use the new |
|
It will be a tricky test since to make it work you need to make allocations that will fail, and only after it compare, but with what? the only thing we can use in the test is the RSS (since amount is not exported), but there is a drift between RSS and |
|
Ok, we can merge it without a test. Let's check the CI failures. |
|
(without a test it does not count as a "Bug Fix") |
CI failed due to |
Changed it to |
MemoryTracking metric takes into account memory allocation even in case
of this allocation will fail with MEMORY_LIMIT_EXCEEDED, which is not
good, which eventually will lead to `amount` and `MemoryTracking`
mismatch, I found one server with **43x difference**.
<details>
```sql
SELECT
event_time_microseconds,
message
FROM system.text_log
WHERE (event_date = (today() - 1)) AND (logger_name = 'MemoryTracker') AND (message LIKE '%total%')
ORDER BY 1 DESC
LIMIT 1
Query id: 64d60852-fa14-4ed1-adb1-d4bbd6159475
┌────event_time_microseconds─┬─message───────────────────────────────────┐
1. │ 2024-11-27 05:09:48.157608 │ Current memory usage (total): 471.00 GiB. │
└────────────────────────────┴───────────────────────────────────────────┘
```
```sql
SELECT
metric,
formatReadableSize(value)
FROM system.metrics
WHERE (metric ILIKE '%mem%') OR (metric ILIKE '%jemalloc%')
ORDER BY value ASC
Query id: af7908a8-956a-4684-b7c5-b2e0c6fa06f4
┌─metric────────────────────────┬─formatReadableSize(value)─┐
1. │ MergesMutationsMemoryTracking │ 0.00 B │
2. │ MemoryTracking │ 20.37 TiB │
└───────────────────────────────┴───────────────────────────┘
```
</details>
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
|
Rebased |
|
Bugfix validation failed, but we set the category "Not for changelog", so it is fine. |
MemoryTracking metric takes into account memory allocation even in case of this allocation will fail with MEMORY_LIMIT_EXCEEDED, which is not good, which eventually will lead to
amountandMemoryTrackingmismatch, I found one server with 43x difference.Details
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix inaccurate
MemoryTrackingmetric in case of allocation failures