This is an automated email from the ASF dual-hosted git repository.
stack pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 7ac1c8b HBASE-25677 Server+table counters on each scan #nextRaw
invocation becomes a bottleneck when heavy load (#3061)
7ac1c8b is described below
commit 7ac1c8bbf8da31586e786819ab515236ce6dbbdc
Author: Michael Stack <saintst...@users.noreply.github.com>
AuthorDate: Thu Mar 18 11:33:45 2021 -0700
HBASE-25677 Server+table counters on each scan #nextRaw invocation becomes
a bottleneck when heavy load (#3061)
Don't have every handler update regionserver metrics on each
scan#nextRaw; instead, do a batch update just before Scan
returns. Otherwise, all running handlers end up contending
on metrics update.
M
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
Update of regionserver metrics counters moved out to caller where
can be done as a batch update instead of per-next.
M
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
Class doc to encourage batch updating metrics.
Remove the single update as unused anymore.
M
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
Count calls to nextRaw. Update regionserver count in finally block when
scan is done rather than per nextRaw call. Move all metrics updates to
finally.
Signed-off-by: Reid Chan <reidc...@apache.org>
Signed-off-by: Baiqiang Zhao <ZhaoBQ>
---
.../hbase/regionserver/MetricsRegionServer.java | 18 ++++++------------
.../hadoop/hbase/regionserver/RSRpcServices.java | 19 +++++++++++++------
.../hadoop/hbase/regionserver/RegionScannerImpl.java | 5 +----
3 files changed, 20 insertions(+), 22 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
index 86b97a2..1d9fb2d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java
@@ -28,12 +28,11 @@ import org.apache.yetus.audience.InterfaceAudience;
import org.apache.yetus.audience.InterfaceStability;
/**
- * <p>
- * This class is for maintaining the various regionserver statistics
- * and publishing them through the metrics interfaces.
- * </p>
+ * Maintains regionserver statistics and publishes them through the metrics
interfaces.
* This class has a number of metrics variables that are publicly accessible;
- * these variables (objects) have methods to update their values.
+ * these variables (objects) have methods to update their values. Batch your
updates rather than
+ * call on each instance else all threads will do nothing but contend trying
to maintain metric
+ * counters!
*/
@InterfaceStability.Evolving
@InterfaceAudience.Private
@@ -52,7 +51,9 @@ public class MetricsRegionServer {
private MetricRegistry metricRegistry;
private Timer bulkLoadTimer;
+ // Incremented once for each call to Scan#nextRaw
private Meter serverReadQueryMeter;
+ // Incremented per write.
private Meter serverWriteQueryMeter;
protected long slowMetricTime;
protected static final int DEFAULT_SLOW_METRIC_TIME = 1000; // milliseconds
@@ -286,13 +287,6 @@ public class MetricsRegionServer {
this.serverReadQueryMeter.mark(count);
}
- public void updateReadQueryMeter(TableName tn) {
- if (tableMetrics != null && tn != null) {
- tableMetrics.updateTableReadQueryMeter(tn);
- }
- this.serverReadQueryMeter.mark();
- }
-
public void updateWriteQueryMeter(TableName tn, long count) {
if (tableMetrics != null && tn != null) {
tableMetrics.updateTableWriteQueryMeter(tn, count);
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index d06778a..5b0b77d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -3361,10 +3361,13 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
// arbitrary 32. TODO: keep record of general size of results being
returned.
List<Cell> values = new ArrayList<>(32);
region.startRegionOperation(Operation.SCAN);
+ long before = EnvironmentEdgeManager.currentTime();
+ // Used to check if we've matched the row limit set on the Scan
+ int numOfCompleteRows = 0;
+ // Count of times we call nextRaw; can be > numOfCompleteRows.
+ int numOfNextRawCalls = 0;
try {
int numOfResults = 0;
- int numOfCompleteRows = 0;
- long before = EnvironmentEdgeManager.currentTime();
synchronized (scanner) {
boolean stale = (region.getRegionInfo().getReplicaId() != 0);
boolean clientHandlesPartials =
@@ -3420,6 +3423,7 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
// Collect values to be returned here
moreRows = scanner.nextRaw(values, scannerContext);
+ numOfNextRawCalls++;
if (!values.isEmpty()) {
if (limitOfRows > 0) {
@@ -3511,18 +3515,21 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
builder.setScanMetrics(metricBuilder.build());
}
}
+ } finally {
+ region.closeRegionOperation();
+ // Update serverside metrics, even on error.
long end = EnvironmentEdgeManager.currentTime();
long responseCellSize = context != null ? context.getResponseCellSize()
: 0;
region.getMetrics().updateScanTime(end - before);
final MetricsRegionServer metricsRegionServer =
regionServer.getMetrics();
if (metricsRegionServer != null) {
metricsRegionServer.updateScanSize(
- region.getTableDescriptor().getTableName(), responseCellSize);
+ region.getTableDescriptor().getTableName(), responseCellSize);
metricsRegionServer.updateScanTime(
- region.getTableDescriptor().getTableName(), end - before);
+ region.getTableDescriptor().getTableName(), end - before);
+
metricsRegionServer.updateReadQueryMeter(region.getRegionInfo().getTable(),
+ numOfNextRawCalls);
}
- } finally {
- region.closeRegionOperation();
}
// coprocessor postNext hook
if (region.getCoprocessorHost() != null) {
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java
index 5d81687..a59f643 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScannerImpl.java
@@ -275,9 +275,6 @@ class RegionScannerImpl implements RegionScanner, Shipper,
RpcCallback {
region.getMetrics().updateReadRequestCount();
}
}
- if (rsServices != null && rsServices.getMetrics() != null) {
- rsServices.getMetrics().updateReadQueryMeter(getRegionInfo().getTable());
- }
// If the size limit was reached it means a partial Result is being
returned. Returning a
// partial Result means that we should not reset the filters; filters
should only be reset in
@@ -779,4 +776,4 @@ class RegionScannerImpl implements RegionScanner, Shipper,
RpcCallback {
// callback
this.close();
}
-}
\ No newline at end of file
+}
