You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2019/10/21 16:22:00 UTC
[hbase] branch branch-1 updated: HBASE-23172 HBase Canary region
success count metrics reflect column family successes, not region successes
This is an automated email from the ASF dual-hosted git repository.
stack pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-1 by this push:
new e183c90 HBASE-23172 HBase Canary region success count metrics reflect column family successes, not region successes
e183c90 is described below
commit e183c90fe9d26b5949ae0c1021be90ee02ed4c46
Author: Caroline Zhou <ca...@salesforce.com>
AuthorDate: Thu Oct 17 19:51:58 2019 -0700
HBASE-23172 HBase Canary region success count metrics reflect column family successes, not region successes
---
.../org/apache/hadoop/hbase/tool/CanaryTool.java | 39 +++++++++++++-------
.../apache/hadoop/hbase/tool/TestCanaryTool.java | 41 +++++++++++++---------
2 files changed, 51 insertions(+), 29 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/tool/CanaryTool.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/tool/CanaryTool.java
index 23daf03..709d22e 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/tool/CanaryTool.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/tool/CanaryTool.java
@@ -269,7 +269,7 @@ public class CanaryTool implements Tool, Canary {
public static class RegionStdOutSink extends StdOutSink {
private Map<String, AtomicLong> perTableReadLatency = new HashMap<>();
private AtomicLong writeLatency = new AtomicLong();
- private final Map<String, RegionTaskResult> regionMap = new ConcurrentHashMap<>();
+ private final Map<String, List<RegionTaskResult>> regionMap = new ConcurrentHashMap<>();
public void publishReadFailure(ServerName serverName, HRegionInfo region, Exception e) {
incReadFailureCount();
@@ -285,10 +285,13 @@ public class CanaryTool implements Tool, Canary {
public void publishReadTiming(ServerName serverName, HRegionInfo region,
HColumnDescriptor column, long msTime) {
+ RegionTaskResult rtr = new RegionTaskResult(region, region.getTable(), serverName, column);
+ rtr.setReadSuccess();
+ rtr.setReadLatency(msTime);
+ List<RegionTaskResult> rtrs = regionMap.get(region.getRegionNameAsString());
+ rtrs.add(rtr);
+ // Note that read success count will be equal to total column family read successes.
incReadSuccessCount();
- RegionTaskResult res = this.regionMap.get(region.getRegionNameAsString());
- res.setReadSuccess();
- res.setReadLatency(msTime);
LOG.info("Read from {} on {} {} in {}ms", region.getRegionNameAsString(), serverName,
column.getNameAsString(), msTime);
}
@@ -307,10 +310,13 @@ public class CanaryTool implements Tool, Canary {
public void publishWriteTiming(ServerName serverName, HRegionInfo region,
HColumnDescriptor column, long msTime) {
+ RegionTaskResult rtr = new RegionTaskResult(region, region.getTable(), serverName, column);
+ rtr.setWriteSuccess();
+ rtr.setWriteLatency(msTime);
+ List<RegionTaskResult> rtrs = regionMap.get(region.getRegionNameAsString());
+ rtrs.add(rtr);
+ // Note that write success count will be equal to total column family write successes.
incWriteSuccessCount();
- RegionTaskResult res = this.regionMap.get(region.getRegionNameAsString());
- res.setWriteSuccess();
- res.setWriteLatency(msTime);
LOG.info("Write to {} on {} {} in {}ms",
region.getRegionNameAsString(), serverName, column.getNameAsString(), msTime);
}
@@ -333,7 +339,7 @@ public class CanaryTool implements Tool, Canary {
return this.writeLatency;
}
- public Map<String, RegionTaskResult> getRegionMap() {
+ public Map<String, List<RegionTaskResult>> getRegionMap() {
return this.regionMap;
}
@@ -1042,15 +1048,17 @@ public class CanaryTool implements Tool, Canary {
private HRegionInfo region;
private TableName tableName;
private ServerName serverName;
+ private HColumnDescriptor column;
private AtomicLong readLatency = null;
private AtomicLong writeLatency = null;
private boolean readSuccess = false;
private boolean writeSuccess = false;
- public RegionTaskResult(HRegionInfo region, TableName tableName, ServerName serverName) {
+ public RegionTaskResult(HRegionInfo region, TableName tableName, ServerName serverName, HColumnDescriptor column) {
this.region = region;
this.tableName = tableName;
this.serverName = serverName;
+ this.column = column;
}
public HRegionInfo getRegionInfo() {
@@ -1077,6 +1085,14 @@ public class CanaryTool implements Tool, Canary {
return this.serverName.getServerName();
}
+ public HColumnDescriptor getColumnFamily() {
+ return this.column;
+ }
+
+ public String getColumnFamilyNameAsString() {
+ return this.column.getNameAsString();
+ }
+
public long getReadLatency() {
if (this.readLatency == null) {
return -1;
@@ -1568,9 +1584,8 @@ public class CanaryTool implements Tool, Canary {
HRegionInfo region = location.getRegionInfo();
tasks.add(new RegionTask(admin.getConnection(), region, rs, (RegionStdOutSink)sink,
taskType, rawScanEnabled, rwLatency));
- Map<String, RegionTaskResult> regionMap = ((RegionStdOutSink) sink).getRegionMap();
- regionMap.put(region.getRegionNameAsString(), new RegionTaskResult(region,
- region.getTable(), rs));
+ Map<String, List<RegionTaskResult>> regionMap = ((RegionStdOutSink) sink).getRegionMap();
+ regionMap.put(region.getRegionNameAsString(), new ArrayList<RegionTaskResult>());
}
return executor.invokeAll(tasks);
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/tool/TestCanaryTool.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/tool/TestCanaryTool.java
index c07e80f..f3fbf16 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/tool/TestCanaryTool.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/tool/TestCanaryTool.java
@@ -34,6 +34,7 @@ import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -139,6 +140,10 @@ public class TestCanaryTool {
String[] args = { "-writeSniffing", "-t", "10000", "testCanaryRegionTaskResult" };
assertEquals(0, ToolRunner.run(testingUtility.getConfiguration(), canary, args));
+ assertTrue("canary should expect to scan at least 1 region",
+ sink.getTotalExpectedRegions() > 0);
+ assertTrue("there should be no read failures", sink.getReadFailureCount() == 0);
+ assertTrue("there should be no write failures", sink.getWriteFailureCount() == 0);
assertTrue("verify read success count > 0", sink.getReadSuccessCount() > 0);
assertTrue("verify write success count > 0", sink.getWriteSuccessCount() > 0);
verify(sink, atLeastOnce()).publishReadTiming(isA(ServerName.class), isA(HRegionInfo.class),
@@ -146,27 +151,29 @@ public class TestCanaryTool {
verify(sink, atLeastOnce()).publishWriteTiming(isA(ServerName.class), isA(HRegionInfo.class),
isA(HColumnDescriptor.class), anyLong());
- assertTrue("canary should expect to scan at least 1 region",
- sink.getTotalExpectedRegions() > 0);
- Map<String, CanaryTool.RegionTaskResult> regionMap = sink.getRegionMap();
+ assertEquals("canary region success count should equal total expected regions",
+ sink.getReadSuccessCount() + sink.getWriteSuccessCount(), sink.getTotalExpectedRegions());
+ Map<String, List<CanaryTool.RegionTaskResult>> regionMap = sink.getRegionMap();
assertFalse("verify region map has size > 0", regionMap.isEmpty());
for (String regionName : regionMap.keySet()) {
- CanaryTool.RegionTaskResult res = regionMap.get(regionName);
- assertNotNull("verify each expected region has a RegionTaskResult object in the map", res);
- assertNotNull("verify getRegionNameAsString()", regionName);
- assertNotNull("verify getRegionInfo()", res.getRegionInfo());
- assertNotNull("verify getTableName()", res.getTableName());
- assertNotNull("verify getTableNameAsString()", res.getTableNameAsString());
- assertNotNull("verify getServerName()", res.getServerName());
- assertNotNull("verify getServerNameAsString()", res.getServerNameAsString());
+ for (CanaryTool.RegionTaskResult res: regionMap.get(regionName)) {
+ assertNotNull("verify getRegionNameAsString()", regionName);
+ assertNotNull("verify getRegionInfo()", res.getRegionInfo());
+ assertNotNull("verify getTableName()", res.getTableName());
+ assertNotNull("verify getTableNameAsString()", res.getTableNameAsString());
+ assertNotNull("verify getServerName()", res.getServerName());
+ assertNotNull("verify getServerNameAsString()", res.getServerNameAsString());
+ assertNotNull("verify getColumnFamily()", res.getColumnFamily());
+ assertNotNull("verify getColumnFamilyNameAsString()", res.getColumnFamilyNameAsString());
- if (regionName.contains(CanaryTool.DEFAULT_WRITE_TABLE_NAME.getNameAsString())) {
- assertTrue("write to region " + regionName + " succeeded", res.isWriteSuccess());
- assertTrue("write took some time", res.getWriteLatency() > -1);
- } else {
- assertTrue("read from region " + regionName + " succeeded", res.isReadSuccess());
- assertTrue("read took some time", res.getReadLatency() > -1);
+ if (regionName.contains(CanaryTool.DEFAULT_WRITE_TABLE_NAME.getNameAsString())) {
+ assertTrue("write to region " + regionName + " succeeded", res.isWriteSuccess());
+ assertTrue("write took some time", res.getWriteLatency() > -1);
+ } else {
+ assertTrue("read from region " + regionName + " succeeded", res.isReadSuccess());
+ assertTrue("read took some time", res.getReadLatency() > -1);
+ }
}
}
}