This is an automated email from the ASF dual-hosted git repository.
reidchan 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 74bb6a7 HBASE-25677 Server+table counters on each scan #nextRaw
invocation becomes a bottleneck when heavy load (#3066)
74bb6a7 is described below
commit 74bb6a7a3aa9278b4d68b191cb900c4b3083d161
Author: YutSean <33572832+yuts...@users.noreply.github.com>
AuthorDate: Wed Mar 24 14:12:51 2021 +0800
HBASE-25677 Server+table counters on each scan #nextRaw invocation becomes
a bottleneck when heavy load (#3066)
Signed-off-by: stack <st...@apache.org>
Signed-off-by: Reid Chan <reidc...@apache.org>
---
.../org/apache/hadoop/hbase/regionserver/HRegion.java | 3 ---
.../hadoop/hbase/regionserver/MetricsRegionServer.java | 18 ++++++------------
.../hadoop/hbase/regionserver/RSRpcServices.java | 15 +++++++++++----
3 files changed, 17 insertions(+), 19 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index 9b28ab2..21cf353 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -6402,9 +6402,6 @@ public class HRegion implements HeapSize,
PropagatingConfigurationObserver, Regi
if (!outResults.isEmpty()) {
readRequestsCount.increment();
}
- 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
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 1f31714..cd725d0 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.hadoop.hbase.metrics.Timer;
import org.apache.hadoop.conf.Configuration;
/**
- * <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
@@ -49,7 +48,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;
public MetricsRegionServer(MetricsRegionServerWrapper regionServerWrapper,
Configuration conf) {
@@ -241,13 +242,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 adef164..a4160f0 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
@@ -2901,10 +2901,13 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
// arbitrary 32. TODO: keep record of general size of results being
returned.
List<Cell> values = new ArrayList<Cell>(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 =
@@ -2958,6 +2961,7 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
// Collect values to be returned here
moreRows = scanner.nextRaw(values, scannerContext);
+ numOfNextRawCalls++;
if (!values.isEmpty()) {
if (limitOfRows > 0) {
@@ -3053,6 +3057,9 @@ 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);
@@ -3061,9 +3068,9 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
region.getTableDesc().getTableName(), responseCellSize);
regionServer.metricsRegionServer.updateScanTime(
region.getTableDesc().getTableName(), end - before);
+ regionServer.metricsRegionServer
+ .updateReadQueryMeter(region.getRegionInfo().getTable(),
numOfNextRawCalls);
}
- } finally {
- region.closeRegionOperation();
}
// coprocessor postNext hook
if (region.getCoprocessorHost() != null) {
