Apache9 commented on code in PR #7363:
URL: https://github.com/apache/hbase/pull/7363#discussion_r2413878784


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncBufferedMutatorImpl.java:
##########
@@ -46,6 +45,8 @@ class AsyncBufferedMutatorImpl implements 
AsyncBufferedMutator {
 
   private static final Logger LOG = 
LoggerFactory.getLogger(AsyncBufferedMutatorImpl.class);
 
+  private final int INITIAL_CAPACITY = 100;

Review Comment:
   static



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncBufferedMutatorImpl.java:
##########
@@ -90,19 +108,41 @@ public Configuration getConfiguration() {
 
   // will be overridden in test
   protected void internalFlush() {
-    if (periodicFlushTask != null) {
-      periodicFlushTask.cancel();
-      periodicFlushTask = null;
-    }
-    List<Mutation> toSend = this.mutations;
-    if (toSend.isEmpty()) {
-      return;
+    internalFlush(FlushType.MANUAL);
+  }
+
+  protected void internalFlush(FlushType trigger) {

Review Comment:
   Maybe a better choice is to inline this method to the caller, and make the 
lock protect the check and replace logic, i.e, if we think we should flush, 
then swap the mutations and futures to local variables and recreate them, and 
set a local bool variable may be called `shouldSend` to true, and check this 
bool outside the lock protection and send the mutate request out.
   
   In this way we do not need the double check and flush type check too, and 
the logic will be more easy to understand. The only problem is the test which 
overrides internalFlush method. We could try to find other ways to implement it.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to