[GitHub] [hbase] busbey commented on a change in pull request #2193: HBASE-24779 Report on the WAL edit buffer usage/limit for replication

[GitBox]

busbey commented on a change in pull request #2193:
URL: https://github.com/apache/hbase/pull/2193#discussion_r465182276



##
File path: 
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationGlobalSourceSource.java
##
@@ -1,253 +1,19 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-

Review comment:
   gotta put this back.





This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [hbase] busbey commented on a change in pull request #2193: HBASE-24779 Report on the WAL edit buffer usage/limit for replication

[GitBox]

busbey commented on a change in pull request #2193:
URL: https://github.com/apache/hbase/pull/2193#discussion_r464770288



##
File path: 
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationSourceSourceImpl.java
##
@@ -314,4 +314,15 @@ public String getMetricsName() {
   @Override public long getEditsFiltered() {
 return this.walEditsFilteredCounter.value();
   }
+
+  @Override
+  public void setWALReaderEditsBufferBytes(long usage) {
+//noop. Global limit, tracked globally. Do not need per-source metrics

Review comment:
   nit: wouldn't it still be useful to know if particular sources were 
eating up the global limit? or do we not have enough information tracked 
already to do that?

##
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationEndpoint.java
##
@@ -497,6 +497,33 @@ public boolean canReplicateToSameCluster() {
 }
   }
 
+  public static class SleepingReplicationEndpointForTest extends 
ReplicationEndpointForTest {

Review comment:
   AFAICT this only gets used in manual testing? add a comment on doing 
that. something like your example for this PR.

##
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationEndpoint.java
##
@@ -497,6 +497,33 @@ public boolean canReplicateToSameCluster() {
 }
   }
 
+  public static class SleepingReplicationEndpointForTest extends 
ReplicationEndpointForTest {
+private long duration;
+public SleepingReplicationEndpointForTest() {
+  super();
+}
+
+@Override
+public void init(Context context) throws IOException {
+  super.init(context);
+  if (this.ctx != null) {
+duration = this.ctx.getConfiguration().getLong(
+"test.sleep.replication.endpoint.duration.millis", 5000L);

Review comment:
   nit: `hbase.test.sleep.replication.endpoint.duration.millis`

##
File path: 
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/replication/regionserver/MetricsReplicationSourceSource.java
##
@@ -76,4 +77,13 @@
   long getWALEditsRead();
   long getShippedOps();
   long getEditsFiltered();
+  /**
+   * Sets the total usage of memory used by edits in memory read from WALs.
+   * @param usage The memory used by edits in bytes
+   */
+  void setWALReaderEditsBufferBytes(long usage);

Review comment:
   if we only want the global metric, why add this here instead of just on 
`MetricsReplicationGlobalSourceSource`?

##
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/Replication.java
##
@@ -244,17 +248,22 @@ void addHFileRefsToQueue(TableName tableName, byte[] 
family, List