You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/24 10:49:26 UTC

[GitHub] [ozone] ChenSammi opened a new pull request, #3875: HDDS-7183. Expose RocksDB critical properties.

ChenSammi opened a new pull request, #3875:
URL: https://github.com/apache/ozone/pull/3875

   https://issues.apache.org/jira/browse/HDDS-7183


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "DaveTeng0 (via GitHub)" <gi...@apache.org>.
DaveTeng0 commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1090190018


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -179,6 +234,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean b) {
     MetricsRecordBuilder rb = metricsCollector.addRecord(contextName);
     getHistogramData(rb);
     getTickerTypeData(rb);
+    getDBPropertyData(rb);

Review Comment:
   hey @ChenSammi ~ just a follow up on this!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1045558207


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -61,22 +69,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
       LoggerFactory.getLogger(RocksDBStoreMBean.class);
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
+  public static final String ROCKSDB_PROPERTY_PREFIX = "rocksdb.";
+
+  // RocksDB properties
+  private final String[][] cfPros = {
+      // 1 if a memtable flush is pending; otherwise, returns 0
+      {"mem-table-flush-pending", "false", ""},
+      // estimated total number of bytes compaction needs to rewrite to get
+      // all levels down to under target size.
+      {"estimate-pending-compaction-bytes", "true", ""},
+      // 1 if at least one compaction is pending; otherwise, returns 0.
+      {"compaction-pending", "false", ""},
+      // block cache capacity
+      {"block-cache-capacity", "true", ""},
+      // the memory size for the entries residing in block cache
+      {"block-cache-usage", "true", ""},
+      // the memory size for the entries being pinned
+      {"block-cache-pinned-usage", "true", ""},
+      // number of level to which L0 data will be compacted.
+      {"base-level", "false", ""},
+      // approximate active mem table size (bytes)
+      {"cur-size-active-mem-table", "true", ""},
+      // approximate size of active and unflushed immutable (bytes)
+      {"cur-size-all-mem-tables", "true", ""},
+      // approximate size of active, unflushed immutable, and pinned immutable
+      // memtables (bytes)
+      {"size-all-mem-tables", "true", ""},
+      // number of immutable memtables that have not yet been flushed
+      {"num-immutable-mem-table", "true", ""},
+      // total size (bytes) of all SST files belong to the latest LSM tree
+      {"live-sst-files-size", "true", ""},
+      // estimated number of total keys in memtables and storage(Can be very
+      // wrong according to RocksDB document)
+      {"estimate-num-keys", "true", ""},
+      // estimated memory used for reading SST tables, excluding memory used
+      // in block cache (e.g., filter and index blocks)
+      {"estimate-table-readers-mem", "true", ""}
+  };
 
-  public RocksDBStoreMBean(Statistics statistics, String dbName) {
+  // level-x sst file info (Global)
+  private static final String NUM_FILES_AT_LEVEL = "num_files_at_level";
+  private static final String SIZE_AT_LEVEL = "size_at_level";
+
+  public RocksDBStoreMBean(Statistics statistics, RocksDatabase db,
+      String dbName) {
     this.contextName = ROCKSDB_CONTEXT_PREFIX + dbName;
     this.statistics = statistics;
+    this.rocksDB = db;
     histogramAttributes.add("Average");
     histogramAttributes.add("Median");
     histogramAttributes.add("Percentile95");
     histogramAttributes.add("Percentile99");
     histogramAttributes.add("StandardDeviation");
+    histogramAttributes.add("Max");
+
+    // To get the metric name complied with prometheus naming rule
+    for (String[] property : cfPros) {
+      property[2] = property[0].replace("-", "_");

Review Comment:
   I checked all the metrics, except HistogramData and TickerTypeData are in upper-case, all other metrics are in lower-case.  So probably we should covert HistogramData and TickerTypeData to lower-case to be consistent. We should do it in another patch. 



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -179,6 +234,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean b) {
     MetricsRecordBuilder rb = metricsCollector.addRecord(contextName);
     getHistogramData(rb);
     getTickerTypeData(rb);
+    getDBPropertyData(rb);

Review Comment:
   Make sense. Let me check the code. 



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -61,22 +69,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
       LoggerFactory.getLogger(RocksDBStoreMBean.class);
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
+  public static final String ROCKSDB_PROPERTY_PREFIX = "rocksdb.";
+
+  // RocksDB properties

Review Comment:
   Sure.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3875:
URL: https://github.com/apache/ozone/pull/3875#issuecomment-1288849184

   Some output example,
   file number per each level
   ![image](https://user-images.githubusercontent.com/19790142/197509733-9376265e-0a24-4ec3-9e3e-9abf38a61f37.png)
   
   total file size per each level
   ![image](https://user-images.githubusercontent.com/19790142/197509855-a1c48d44-a467-4899-a4c1-a8010dfc84ea.png)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #3875:
URL: https://github.com/apache/ozone/pull/3875#issuecomment-1448030342

   Thanks @symious for the code review.  I opened HDDS-8043 to track the task to enable rocksdb statistics by default. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi merged pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #3875:
URL: https://github.com/apache/ozone/pull/3875


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1116596235


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -179,6 +234,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean b) {
     MetricsRecordBuilder rb = metricsCollector.addRecord(contextName);
     getHistogramData(rb);
     getTickerTypeData(rb);
+    getDBPropertyData(rb);

Review Comment:
   @symious ,  some newly added metrics, for example "estimate-pending-compaction-bytes". If the value is too high, it 's a warning message. To find out why there are so many pending compaction bytes, we need to look into other rocksdb metrics, which are exposed by HistogramData and TickerTypeData.  
   
   So although the newly added properties in patch doesn't depend on rocksdb.statistics turn on, but in a real problem investigation, all these three types of metrics information are all needed. So rocksdb.statistics controls all metrics display is a fair solution. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #3875:
URL: https://github.com/apache/ozone/pull/3875#issuecomment-1435731888

   /pending latest comments by @symious should be addressed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1046683249


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -61,22 +69,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
       LoggerFactory.getLogger(RocksDBStoreMBean.class);
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
+  public static final String ROCKSDB_PROPERTY_PREFIX = "rocksdb.";
+
+  // RocksDB properties
+  private final String[][] cfPros = {
+      // 1 if a memtable flush is pending; otherwise, returns 0
+      {"mem-table-flush-pending", "false", ""},
+      // estimated total number of bytes compaction needs to rewrite to get
+      // all levels down to under target size.
+      {"estimate-pending-compaction-bytes", "true", ""},
+      // 1 if at least one compaction is pending; otherwise, returns 0.
+      {"compaction-pending", "false", ""},
+      // block cache capacity
+      {"block-cache-capacity", "true", ""},
+      // the memory size for the entries residing in block cache
+      {"block-cache-usage", "true", ""},
+      // the memory size for the entries being pinned
+      {"block-cache-pinned-usage", "true", ""},
+      // number of level to which L0 data will be compacted.
+      {"base-level", "false", ""},
+      // approximate active mem table size (bytes)
+      {"cur-size-active-mem-table", "true", ""},
+      // approximate size of active and unflushed immutable (bytes)
+      {"cur-size-all-mem-tables", "true", ""},
+      // approximate size of active, unflushed immutable, and pinned immutable
+      // memtables (bytes)
+      {"size-all-mem-tables", "true", ""},
+      // number of immutable memtables that have not yet been flushed
+      {"num-immutable-mem-table", "true", ""},
+      // total size (bytes) of all SST files belong to the latest LSM tree
+      {"live-sst-files-size", "true", ""},
+      // estimated number of total keys in memtables and storage(Can be very
+      // wrong according to RocksDB document)
+      {"estimate-num-keys", "true", ""},
+      // estimated memory used for reading SST tables, excluding memory used
+      // in block cache (e.g., filter and index blocks)
+      {"estimate-table-readers-mem", "true", ""}
+  };
 
-  public RocksDBStoreMBean(Statistics statistics, String dbName) {
+  // level-x sst file info (Global)
+  private static final String NUM_FILES_AT_LEVEL = "num_files_at_level";
+  private static final String SIZE_AT_LEVEL = "size_at_level";
+
+  public RocksDBStoreMBean(Statistics statistics, RocksDatabase db,
+      String dbName) {
     this.contextName = ROCKSDB_CONTEXT_PREFIX + dbName;
     this.statistics = statistics;
+    this.rocksDB = db;
     histogramAttributes.add("Average");
     histogramAttributes.add("Median");
     histogramAttributes.add("Percentile95");
     histogramAttributes.add("Percentile99");
     histogramAttributes.add("StandardDeviation");
+    histogramAttributes.add("Max");
+
+    // To get the metric name complied with prometheus naming rule
+    for (String[] property : cfPros) {
+      property[2] = property[0].replace("-", "_");

Review Comment:
   OK.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1045097934


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -61,22 +69,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
       LoggerFactory.getLogger(RocksDBStoreMBean.class);
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
+  public static final String ROCKSDB_PROPERTY_PREFIX = "rocksdb.";
+
+  // RocksDB properties
+  private final String[][] cfPros = {
+      // 1 if a memtable flush is pending; otherwise, returns 0
+      {"mem-table-flush-pending", "false", ""},
+      // estimated total number of bytes compaction needs to rewrite to get
+      // all levels down to under target size.
+      {"estimate-pending-compaction-bytes", "true", ""},
+      // 1 if at least one compaction is pending; otherwise, returns 0.
+      {"compaction-pending", "false", ""},
+      // block cache capacity
+      {"block-cache-capacity", "true", ""},
+      // the memory size for the entries residing in block cache
+      {"block-cache-usage", "true", ""},
+      // the memory size for the entries being pinned
+      {"block-cache-pinned-usage", "true", ""},
+      // number of level to which L0 data will be compacted.
+      {"base-level", "false", ""},
+      // approximate active mem table size (bytes)
+      {"cur-size-active-mem-table", "true", ""},
+      // approximate size of active and unflushed immutable (bytes)
+      {"cur-size-all-mem-tables", "true", ""},
+      // approximate size of active, unflushed immutable, and pinned immutable
+      // memtables (bytes)
+      {"size-all-mem-tables", "true", ""},
+      // number of immutable memtables that have not yet been flushed
+      {"num-immutable-mem-table", "true", ""},
+      // total size (bytes) of all SST files belong to the latest LSM tree
+      {"live-sst-files-size", "true", ""},
+      // estimated number of total keys in memtables and storage(Can be very
+      // wrong according to RocksDB document)
+      {"estimate-num-keys", "true", ""},
+      // estimated memory used for reading SST tables, excluding memory used
+      // in block cache (e.g., filter and index blocks)
+      {"estimate-table-readers-mem", "true", ""}
+  };
 
-  public RocksDBStoreMBean(Statistics statistics, String dbName) {
+  // level-x sst file info (Global)
+  private static final String NUM_FILES_AT_LEVEL = "num_files_at_level";
+  private static final String SIZE_AT_LEVEL = "size_at_level";
+
+  public RocksDBStoreMBean(Statistics statistics, RocksDatabase db,
+      String dbName) {
     this.contextName = ROCKSDB_CONTEXT_PREFIX + dbName;
     this.statistics = statistics;
+    this.rocksDB = db;
     histogramAttributes.add("Average");
     histogramAttributes.add("Median");
     histogramAttributes.add("Percentile95");
     histogramAttributes.add("Percentile99");
     histogramAttributes.add("StandardDeviation");
+    histogramAttributes.add("Max");
+
+    // To get the metric name complied with prometheus naming rule
+    for (String[] property : cfPros) {
+      property[2] = property[0].replace("-", "_");

Review Comment:
   The metrics of `HistogramData` and `TickerTypeData` are in upper-case, should we also keep the format for the new metrics?



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -61,22 +69,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
       LoggerFactory.getLogger(RocksDBStoreMBean.class);
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
+  public static final String ROCKSDB_PROPERTY_PREFIX = "rocksdb.";
+
+  // RocksDB properties

Review Comment:
   I think it might be more readable to add some comments showing the structure of the `String[][]`, for example, column 1 means ..., column 2 means.... .



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -179,6 +234,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean b) {
     MetricsRecordBuilder rb = metricsCollector.addRecord(contextName);
     getHistogramData(rb);
     getTickerTypeData(rb);
+    getDBPropertyData(rb);

Review Comment:
   When `ozone.metastore.rocksdb.statistics` is set to `OFF` (by default), these metrics won't be available.
   
   Understand that `HistogramData` and `TickerTypeData` are retrieved from `rocksdb.statistics`, so when config if set to "OFF", metrics won't be visible. But seems `DBPropertyData` are independent, should we skip the check on the config?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] myskov commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
myskov commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1003225424


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -62,21 +71,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
 
-  public RocksDBStoreMBean(Statistics statistics, String dbName) {
+  // RocksDB properties
+  String [][] cfPros = {
+      // 1 if a memtable flush is pending; otherwise, returns 0
+      {"mem-table-flush-pending", "false"},
+      // estimated total number of bytes compaction needs to rewrite to get
+      // all levels down to under target size.
+      {"estimate-pending-compaction-bytes", "true"},
+      // 1 if at least one compaction is pending; otherwise, returns 0.
+      {"compaction-pending", "false"},
+      // block cache capacity
+      {"block-cache-capacity", "true"},
+      // the memory size for the entries residing in block cache
+      {"block-cache-usage", "true"},
+      // the memory size for the entries being pinned
+      {"block-cache-pinned-usage", "true"},
+      // number of level to which L0 data will be compacted.
+      {"base-level", "false"},
+      // approximate active mem table size (bytes)
+      {"cur-size-active-mem-table", "true"},
+      // approximate size of active and unflushed immutable (bytes)
+      {"cur-size-all-mem-tables", "true"},
+      // approximate size of active, unflushed immutable, and pinned immutable
+      // memtables (bytes)
+      {"size-all-mem-tables", "true"},
+      // number of immutable memtables that have not yet been flushed
+      {"num-immutable-mem-table", "true"},
+      // total size (bytes) of all SST files belong to the latest LSM tree
+      {"live-sst-files-size", "true"},
+      // estimated number of total keys in memtables and storage(Can be very
+      // wrong according to RocksDB document)
+      {"estimate-num-keys", "true"},
+      // estimated memory used for reading SST tables, excluding memory used
+      // in block cache (e.g., filter and index blocks)
+      {"estimate-table-readers-mem", "true"}
+  };
+
+  // level-x sst file info (Global)
+  String numFilesPerLevel = "num_files_at_level";

Review Comment:
   look like a constant. Should be something linke private final static ROCKSDB_PROP_NUM_FILES_PER_LEVEL



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -62,21 +71,70 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
 
   public static final String ROCKSDB_CONTEXT_PREFIX = "Rocksdb_";
 
-  public RocksDBStoreMBean(Statistics statistics, String dbName) {
+  // RocksDB properties
+  String [][] cfPros = {
+      // 1 if a memtable flush is pending; otherwise, returns 0
+      {"mem-table-flush-pending", "false"},
+      // estimated total number of bytes compaction needs to rewrite to get
+      // all levels down to under target size.
+      {"estimate-pending-compaction-bytes", "true"},
+      // 1 if at least one compaction is pending; otherwise, returns 0.
+      {"compaction-pending", "false"},
+      // block cache capacity
+      {"block-cache-capacity", "true"},
+      // the memory size for the entries residing in block cache
+      {"block-cache-usage", "true"},
+      // the memory size for the entries being pinned
+      {"block-cache-pinned-usage", "true"},
+      // number of level to which L0 data will be compacted.
+      {"base-level", "false"},
+      // approximate active mem table size (bytes)
+      {"cur-size-active-mem-table", "true"},
+      // approximate size of active and unflushed immutable (bytes)
+      {"cur-size-all-mem-tables", "true"},
+      // approximate size of active, unflushed immutable, and pinned immutable
+      // memtables (bytes)
+      {"size-all-mem-tables", "true"},
+      // number of immutable memtables that have not yet been flushed
+      {"num-immutable-mem-table", "true"},
+      // total size (bytes) of all SST files belong to the latest LSM tree
+      {"live-sst-files-size", "true"},
+      // estimated number of total keys in memtables and storage(Can be very
+      // wrong according to RocksDB document)
+      {"estimate-num-keys", "true"},
+      // estimated memory used for reading SST tables, excluding memory used
+      // in block cache (e.g., filter and index blocks)
+      {"estimate-table-readers-mem", "true"}
+  };
+
+  // level-x sst file info (Global)
+  String numFilesPerLevel = "num_files_at_level";
+  String sizePerLevel = "size_at_level";
+
+  // To be Prometheus metric name format
+  String [] formattedCFProps;
+
+  public RocksDBStoreMBean(Statistics statistics, RocksDatabase db,
+      String dbName) {
     this.contextName = ROCKSDB_CONTEXT_PREFIX + dbName;
     this.statistics = statistics;
+    this.rocksDB = db;
     histogramAttributes.add("Average");
     histogramAttributes.add("Median");
     histogramAttributes.add("Percentile95");
     histogramAttributes.add("Percentile99");
     histogramAttributes.add("StandardDeviation");
+    histogramAttributes.add("Max");
+
+    List<String> tempList = new ArrayList<>();
+    Arrays.stream(cfPros).forEach(p -> tempList.add(p[0].replace("-", "_")));
+    formattedCFProps = tempList.toArray(new String[0]);

Review Comment:
   No need for additional tempList(). Can be done with 
   formattedCFProps = Arrays.stream(cfPros).map(x -> x[0].replace("-", "_")).toArray(new String[0])
   



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -53,6 +60,8 @@ public class RocksDBStoreMBean implements DynamicMBean, MetricsSource {
 
   private Statistics statistics;
 
+  private RocksDatabase rocksDB;

Review Comment:
   can be final



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3875:
URL: https://github.com/apache/ozone/pull/3875#issuecomment-1294420686

   Thanks @myskov for the code review. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on a diff in pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #3875:
URL: https://github.com/apache/ozone/pull/3875#discussion_r1117125884


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/RocksDBStoreMBean.java:
##########
@@ -179,6 +234,7 @@ public void getMetrics(MetricsCollector metricsCollector, boolean b) {
     MetricsRecordBuilder rb = metricsCollector.addRecord(contextName);
     getHistogramData(rb);
     getTickerTypeData(rb);
+    getDBPropertyData(rb);

Review Comment:
   I'm not sure about this.
   
   I reckon the default value of "ozone.metastore.rocksdb.statistics" was set to "false" because of some performance impact. But since the newly added metrics don't have such impacts, I think it's better to have these metrics exposed by default.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] symious commented on pull request #3875: HDDS-7183. Expose RocksDB critical properties.

Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on PR #3875:
URL: https://github.com/apache/ozone/pull/3875#issuecomment-1447589310

   As discussed with @ChenSammi, the configuration of "ozone.metastore.rocksdb.statistics" was set to "false" by default due to too many rocksdbs in Container Schema V2. But now with Container Schema V3, we have a smaller amount of rocksdbs, so it's ok to set the configuration to true by default.
   
   @ChenSammi will create another ticket for the change of default value of "ozone.metastore.rocksdb.statistics".
   So, for this ticket, it's ok to be merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org