[GitHub] [hbase] Apache9 commented on a change in pull request #2675: HBASE-25277 postScannerFilterRow impacts Scan performance a lot in HBase 2.x

[GitBox <gi...@apache.org>]

Apache9 commented on a change in pull request #2675:
URL: https://github.com/apache/hbase/pull/2675#discussion_r537036842



##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestRegionCoprocessorHost.java
##########
@@ -79,19 +81,36 @@
 
   @Before
   public void setup() throws IOException {
+    init(null);
+  }
+
+  public void init(Boolean flag) throws IOException {

Review comment:
       Let's use private here? I do not think this method needs to be called in other classes?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
##########
@@ -102,6 +102,11 @@
   // optimization: no need to call postScannerFilterRow, if no coprocessor implements it
   private final boolean hasCustomPostScannerFilterRow;
 
+  @VisibleForTesting

Review comment:
       OK, VisibleForTesting...
   
   We have a dicussion thread in the mailing list, and @apurtell concluded that we do not use annotation at all, just use a javadoc, and had already done a PR to remove all the VisibleForTesting annotations from our code base. I'm still trying to convince him to allow this annotation on IA.Private classes.
   
   There is still no consensus yet, you can see the discussion in this PR
   https://github.com/apache/hbase/pull/2714
   
   So here I suggest that we remove this annotation to align with the current rule in our code base first. But anyway, I think this is an example that developers want to use this annotation, so maybe @apurtell could change his opinion.
   
   Thanks.




----------------------------------------------------------------
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:
users@infra.apache.org