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
HDDS-1568 : Add RocksDB metrics to OM. #868
Conversation
/label ozone |
💔 -1 overall
This message was automatically generated. |
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.
Thanks for getting this change done. I am fine with this change if we are able to work with older YARN installs. You might want to ask Namit if he remembers the details of that issue, or Arpit or elek.
@@ -41,13 +49,21 @@ | |||
/** | |||
* Adapter JMX bean to publish all the Rocksdb metrics. | |||
*/ | |||
public class RocksDBStoreMBean implements DynamicMBean { | |||
public class RocksDBStoreMBean implements DynamicMBean, MetricsSource { | |||
|
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.
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.
@anuengineer I verified by checking the /prom servlet end point. I also made changes in the prometheus sink metric name sanitization code so that the RocksDB metric names don't look odd.
option = dbProfile.getDBOptions(); | ||
} | ||
|
||
if (!rocksDbStat.equals(OZONE_METADATA_STORE_ROCKSDB_STATISTICS_OFF)) { |
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.
There is some history here. During our first release we found that RocksDB is also shipped by YARN. That version of RocksDB is very old, hence this call would fail in mysterious ways. @arp7 went and fixed that issue and made sure that we don't enable this by default. I am fine with enabling this, if we don't run into that old issue again. @elek, @arp7 any comments ?
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.
@anuengineer We don't enable the metrics by default. I just used the same config that is used to enable metrics for SCM RocksDB, to enable metrics for OM RocksDB as well.
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 am not familiar with the old issue, but now our classpath is separated from yarn. We can use any newer rocksdb (but we should use the same versions for all the hdds/ozone projects)
I think it's safe to use any new rocksdb feature if you think metrics should be turned on by default...
💔 -1 overall
This message was automatically generated. |
Causing Travis-CI builds to appear broken on master. Author: Daniel Nishimura <dnishimura@linkedin.com> Reviewers: Prateek Maheshwari <pmaheshwari@apache.org> Closes apache#868 from dnishimura/disable-sonar-scanner
RocksDB statistics need to sinked to hadoop-metrics2 for Ozone Manager to understand how OM behaves under heavy load.
Example: "rocksdb.bytes.written"