bulk: Add metrics to bulk mem monitor for better visualisation #39754
Conversation
|
This change is |
|
A quick test on TPCC-100 allowing cockroach to use 50% of my total memory (8 GiB), shows that the bulk memory monitor reports a max of 2.3GiB was reserved at one point by the 9 concurrent imports. The above graph is a plot of the bytes allocated by the parent I'll run some more tests on larger imports and post my findings. |
|
I don't really know much about the monitors or their API so I'll defer review to someone who does. is that @knz? |
|
@knz I was also a little confused about why we record the logarithmic value of the high watermark of bytes allocated in a monitor. (https://github.com/cockroachdb/cockroach/blob/master/pkg/util/mon/bytes_usage.go#L404) I consulted your comment but can you give me some intuition as to why we would not record the actual value instead? |
adityamaru27
force-pushed
the
monitor-cleanup
branch
from
4d6998b
to
4cfb2a3
Compare
There was a problem hiding this comment.
I consulted your comment but can you give me some intuition as to why we would not record the actual value instead?
Back then the plotting facility in the web UI was not able to plot with a log scale on the y-axis. Given that the sizes of memory allocations follow a power law distribution, the visual result would be a few spikes at a dozen megabyte+ sizes, with the bulk of allocations (<1000 bytes) completely squashed along the x axis.
your implementation LGTM
Reviewed 4 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @adityamaru27)
pkg/util/mon/bytes_usage.go, line 218 at r1 (raw file):
noteworthyUsageBytes int64 curBytesCount *metric.Gauge
These two fields are missing a comment. Please explain what they are and what is their purpose; otherwise the review is difficult.
Note: I do not know whether this has changed. It may still well be the case. |
This change reworks how we want to use memory monitors for bulk operations. We have a single `bulkMonitor` which is a child of the root sql memory monitor. Each bulk operation will further create a child of the `bulkMonitor` which will be used to control memory growth of the bulk adder. A new class of metrics has been hooked up to the `bulkMonitor` which allows for visualization of total memory usage in the admin UI. Release note: None
adityamaru27
force-pushed
the
monitor-cleanup
branch
from
4cfb2a3
to
57ca960
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/util/mon/bytes_usage.go, line 218 at r1 (raw file):
Previously, knz (kena) wrote…
These two fields are missing a comment. Please explain what they are and what is their purpose; otherwise the review is difficult.
Done.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status:
|
TFTR |
39754: bulk: Add metrics to bulk mem monitor for better visualisation r=adityamaru27 a=adityamaru27 This change reworks how we want to use memory monitors for bulk operations. We have a single `bulkMonitor` which is a child of the root sql memory monitor. Each bulk operation will further create a child of the `bulkMonitor` which will be used to control memory growth of the bulk adder. A new class of metrics has been hooked up to the `bulkMonitor` which allows for visualization of total memory usage in the admin UI. Release note: None Co-authored-by: Aditya Maru <adityamaru@cockroachlabs.com>
Build succeeded |
This change reworks how we want to use memory monitors for bulk
operations.
We have a single
bulkMonitorwhich is a child of theroot sql memory monitor. Each bulk operation will further create a
child of the
bulkMonitorwhich will be used to control memorygrowth of the bulk adder.
A new class of metrics has been hooked up to the
bulkMonitorwhich allows for visualization of total memory usage in the
admin UI.
Release note: None