-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-28845 table level wal appendSize and replication source metrics… #6259
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
baf8457
to
3058da9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3058da9
to
fb25735
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
fb25735
to
c96894c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
… not correctly shown in /jmx response xx
c96894c
to
4bbf647
Compare
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -260,6 +260,10 @@ public String getQualifierAsString() { | |||
return qualifierAsString; | |||
} | |||
|
|||
public String getMetricPrefixTableName() { |
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.
TableName is IA.Public so we'd better not add new method unless you are sure it is useful for everyone.
@@ -88,7 +88,7 @@ public void incrementAppendSize(TableName tableName, long size) { | |||
if (tableAppendSizeCounter == null) { | |||
// Ideally putIfAbsent is atomic and we don't need a branch check but we still do it to avoid | |||
// expensive string construction for every append. | |||
String metricsKey = String.format("%s.%s", tableName, APPEND_SIZE); | |||
String metricsKey = String.format("%s.%s", tableName.getMetricPrefixTableName(), APPEND_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.
So the problem here is that, if we have ':' in th table name, it can not be displayed in jmx?
But this is still a breaking change... do we have other ways to obtain the metrics beside jmx? Do these ways also require that there is no ':' in the metrics name?
Found 2 metrics did not display in the /jmx http interface response, table level wal appendSize and table level replication source.
I suspect it's because the metric name contains a colon :
after modify the table name string to "Namespace_$namespace_table_$table, the metric really display correctly in the /jmx response