huaxiangsun commented on a change in pull request #3495:
URL: https://github.com/apache/hbase/pull/3495#discussion_r672393896



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -175,23 +178,17 @@ private void release(int mask) {
     }
   }
 
-  @Override
-  public void cleanup() {
-    release(0b01);
+  public void retainByWAL() {
+    reference.incrementAndGet();
   }
 
-  public void retainByWAL() {
-    for (;;) {
-      int ref = reference.get();
-      int nextRef = ref | 0b10;
-      if (reference.compareAndSet(ref, nextRef)) {
-        return;
+  public void releaseByWAL() {
+    if (reference.decrementAndGet() == 0) {

Review comment:
       I did some test to show the binary bits are set correctly.

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/ServerCall.java
##########
@@ -97,11 +97,13 @@
   private long exceptionSize = 0;
   private final boolean retryImmediatelySupported;
 
-  // This is a dirty hack to address HBASE-22539. The lowest bit is for normal 
rpc cleanup, and the
-  // second bit is for WAL reference. We can only call release if both of them 
are zero. The reason
-  // why we can not use a general reference counting is that, we may call 
cleanup multiple times in
-  // the current implementation. We should fix this in the future.
-  private final AtomicInteger reference = new AtomicInteger(0b01);
+  // This is a dirty hack to address HBASE-22539. The highest bit is for rpc 
ref and cleanup, and
+  // the rest of the bits are for WAL reference count. We can only call 
release if all of them are
+  // zero. The reason why we can not use a general reference counting is that, 
we may call cleanup
+  // multiple times in the current implementation. We should fix this in the 
future.
+  // The refCount here will start as 0x80000000 and increment with every WAL 
reference and decrement
+  // from WAL side on release
+  private final AtomicInteger reference = new AtomicInteger(0x80000000);

Review comment:
       Like this idea. This new class needs to be generic as not only in this 
rpc handler/wal subsystem needs to coordinate. The replication path also needs 
similar coordination. 




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