Add GC metrics #407

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:feat/gc-metrics into master 2023-05-31 10:22:14 +00:00

Closes #376

Signed-off-by: Dmitrii Stepanov d.stepanov@yadro.com

Closes #376 Signed-off-by: Dmitrii Stepanov <d.stepanov@yadro.com>
dstepanov-yadro reviewed 2023-05-29 14:36:07 +00:00
@ -0,0 +40,4 @@
func newGCMetrics() *gcMetrics {
return &gcMetrics{
runDuration: newCounterVec(prometheus.CounterOpts{
Author
Member

Counter vector used for duration because

  1. GC can run 1 second or more than 1 minute, so using buckets will give us less information than counter

  2. GC runs as single process in the moment for shard

Counter vector used for duration because 1. GC can run 1 second or more than 1 minute, so using buckets will give us less information than counter 2. GC runs as single process in the moment for shard
dstepanov-yadro requested review from storage-core-committers 2023-05-29 14:36:21 +00:00
dstepanov-yadro requested review from storage-core-developers 2023-05-29 14:36:21 +00:00
fyrchik reviewed 2023-05-29 14:51:35 +00:00
@ -108,0 +108,4 @@
shard.WithGCMetrics(
&gcMetrics{
storage: e.metrics.GC(),
shardID: id.String(),
Owner

What about extending shard metrics interface with a GC() method?

What about extending shard metrics interface with a `GC()` method?
Author
Member

Shard metrics look too complex already i think. So I would leave it like that, if you don't insist.

Shard metrics look too complex already i think. So I would leave it like that, if you don't insist.
Owner

I mean the choice is between having a single "metrics" interface or multiple metric-related parameters, which feels klunky.
I don't insist, though.

I mean the choice is between having a single "metrics" interface or multiple metric-related parameters, which feels klunky. I don't insist, though.
fyrchik marked this conversation as resolved
@ -0,0 +61,4 @@
inhumedCounter: newCounterVec(prometheus.CounterOpts{
Namespace: namespace,
Subsystem: gcSubsystem,
Name: "marked_to_removal_objects_count",
Owner

for removal?

_for_ removal?
Owner

Also, I think using inhumed_objects_count is also fine.

Also, I think using `inhumed_objects_count` is also fine.
Author
Member

inhumed_objects_count was the first option. But in spec it calls mark

`inhumed_objects_count` was the first option. But in spec it calls `mark`
Author
Member

for removal - fixed

for removal - fixed
dstepanov-yadro force-pushed feat/gc-metrics from 4a8ff6136f to 5c8e6215f8 2023-05-29 15:23:36 +00:00 Compare
acid-ant approved these changes 2023-05-29 19:16:23 +00:00
fyrchik approved these changes 2023-05-31 10:20:54 +00:00
fyrchik approved these changes 2023-05-31 10:22:09 +00:00
fyrchik merged commit 3220c4df9f into master 2023-05-31 10:22:14 +00:00
Sign in to join this conversation.
No reviewers
TrueCloudLab/storage-core-developers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#407
No description provided.