-
Notifications
You must be signed in to change notification settings - Fork 459
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
Prometheus metrics improvements #5570
Conversation
|
||
Db.Metrics.DbSize = _api.DbProvider!.RegisteredDbs.Values.Aggregate(0L, (sum, db) => sum + db.GetSize()); | ||
} | ||
catch (Exception e) |
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.
Why this is general Exception
and what are the options? In other words Why can try fail?
@@ -230,6 +230,8 @@ protected internal void UpdateWriteMetrics() | |||
Metrics.OtherDbWrites++; | |||
} | |||
|
|||
public long GetSize() => long.Parse(_db.GetProperty("rocksdb.total-sst-files-size")); |
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.
How long does it take to do 1000 of such gets while DB is being modified?
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.
I didn't measure. But it's executed once per 5 seconds. So it shouldn't affect overall performance.
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.
If it would take 0.5 seconds or be heavy on disc IO it would, please measure.
Fixes Closes Resolves #
#5553
Changes
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?