Fix visible drift of MemoryTracking metric#10496
Conversation
|
We need to document current approach of memory tracking. Both in parts about limits and metrics. |
| /// Otherwise it might be calculated incorrectly - it can include a "drift" of memory amount. | ||
| /// See https://github.com/ClickHouse/ClickHouse/issues/10293 | ||
| total_memory_tracker.set(data.resident); | ||
| CurrentMetrics::set(CurrentMetrics::MemoryTracking, data.resident); |
There was a problem hiding this comment.
There are still some issues with memory tracking, but now with per-user tracking:
2020.07.06 08:59:07.817896 [ 47193 ] {902cf22b-87e0-4dae-a687-ecd89ba5360c} executeQuery: Code: 241, e.displayText() = DB::Exception: Memory limit (for user) exceeded: would use 437.72 GiB (attempt to allocate chunk of 4200926 bytes), maximum: 437.72 GiB (version 20.6.1.1) (from 10.7.140.7:31318)
Although the server is mostly idle:
SELECT formatReadableSize(memory_usage)
FROM system.processes
SETTINGS max_memory_usage_for_user = 1
┌─formatReadableSize(memory_usage)─┐
│ 289.28 MiB │
│ 155.75 MiB │
│ 0.00 B │
└──────────────────────────────────┘
3 rows in set. Elapsed: 0.002 sec.
@alexey-milovidov how about reseting per-user accounting here too as a temporary solution?
P.S. this version is compiled from sources 811d124 + some of PRs applied (mostly mine), although this should not affect memory tracking.
There was a problem hiding this comment.
@alexey-milovidov FWIW it will be nice to print not only per-user limit on error, but per-query too, and looks like it is not that complex (just adding childs into the MemoryTracker), thoughts?
There was a problem hiding this comment.
how about reseting per-user accounting here too as a temporary solution?
Ok.
FWIW it will be nice to print not only per-user limit on error, but per-query too, and looks like it is not that complex (just adding childs into the MemoryTracker), thoughts?
Ok, that will be better.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Addition to #10293