Skip to content

HBASE-28845 table level wal appendSize and replication source metrics… #6259

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

HBASE-28845 table level wal appendSize and replication source metrics… #6259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ public String getQualifierAsString() {
return qualifierAsString;
}

public String getMetricPrefixTableName() {
Copy link
Contributor

@Apache9 Apache9 Oct 17, 2024

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.

return "Namespace_" + this.getNamespaceAsString() + "table_" + this.getQualifierAsString();
}

/** Returns A pointer to TableName as String bytes. */
public byte[] toBytes() {
return name;
Expand Down
5 changes: 3 additions & 2 deletions ...p-compat/src/main/java/org/apache/hadoop/hbase/regionserver/wal/MetricsWALSourceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

@Apache9 Apache9 Oct 17, 2024

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?

Copy link
Contributor Author

@terrytlu terrytlu Jan 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I tried jmxterm tool, found we could get this metric correctly 😂 ...
Also found this issue already fixed https://issues.apache.org/jira/browse/HBASE-26745, I will close this issue as it is duplicated and already fixed by HBASE-26745.

perTableAppendSize.putIfAbsent(tableName,
getMetricsRegistry().newCounter(metricsKey, APPEND_SIZE_DESC, 0L));
tableAppendSizeCounter = perTableAppendSize.get(tableName);
Expand All @@ -106,7 +106,8 @@ public void incrementAppendCount(TableName tableName) {
appendCount.incr();
MutableFastCounter tableAppendCounter = perTableAppendCount.get(tableName);
if (tableAppendCounter == null) {
String metricsKey = String.format("%s.%s", tableName, APPEND_COUNT);
String metricsKey =
String.format("%s.%s", tableName.getMetricPrefixTableName(), APPEND_COUNT);
perTableAppendCount.putIfAbsent(tableName,
getMetricsRegistry().newCounter(metricsKey, APPEND_COUNT_DESC, 0L));
tableAppendCounter = perTableAppendCount.get(tableName);
Expand Down
2 changes: 1 addition & 1 deletion ...-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void updateTableLevelMetrics(List<Pair<Entry, Long>> walEntries) {
for (Pair<Entry, Long> walEntryWithSize : walEntries) {
Entry entry = walEntryWithSize.getFirst();
long entrySize = walEntryWithSize.getSecond();
String tableName = entry.getKey().getTableName().getNameAsString();
String tableName = entry.getKey().getTableName().getMetricPrefixTableName();
long writeTime = entry.getKey().getWriteTime();
long age = EnvironmentEdgeManager.currentTime() - writeTime;

Expand Down
10 changes: 6 additions & 4 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestMetricsWAL.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,13 @@ public void testPerTableWALMetrics() throws Exception {
// Validate the metrics
for (int i = 0; i < numThreads; i++) {
TableName tableName = TableName.valueOf("tab_" + i);
long tableAppendCount =
registry.getCounter(tableName + "." + MetricsWALSource.APPEND_COUNT, -1).value();
long tableAppendCount = registry
.getCounter(tableName.getMetricPrefixTableName() + "." + MetricsWALSource.APPEND_COUNT, -1)
.value();
assertEquals(numIters, tableAppendCount);
long tableAppendSize =
registry.getCounter(tableName + "." + MetricsWALSource.APPEND_SIZE, -1).value();
long tableAppendSize = registry
.getCounter(tableName.getMetricPrefixTableName() + "." + MetricsWALSource.APPEND_SIZE, -1)
.value();
assertEquals(i * numIters, tableAppendSize);
}
}
Expand Down
7 changes: 5 additions & 2 deletions hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,12 @@ public void testMetricsSourceBaseSourcePassThrough() {
source.getSingleSourceSourceByTable().containsKey("RandomNewTable");
Assert.assertEquals(false, containsRandomNewTable);
source.updateTableLevelMetrics(createWALEntriesWithSize("RandomNewTable"));
containsRandomNewTable = source.getSingleSourceSourceByTable().containsKey("RandomNewTable");
TableName tableName1 = TableName.valueOf("RandomNewTable");
containsRandomNewTable =
source.getSingleSourceSourceByTable().containsKey(tableName1.getMetricPrefixTableName());
Assert.assertEquals(true, containsRandomNewTable);
MetricsReplicationTableSource msr = source.getSingleSourceSourceByTable().get("RandomNewTable");
MetricsReplicationTableSource msr =
source.getSingleSourceSourceByTable().get(tableName1.getMetricPrefixTableName());

// age should be greater than zero we created the entry with time in the past
Assert.assertTrue(msr.getLastShippedAge() > 0);
Expand Down