[GitHub] [hive] pvary commented on a change in pull request #1951: HIVE-24736

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

pvary commented on a change in pull request #1951:
URL: https://github.com/apache/hive/pull/1951#discussion_r571912244



##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java
##########
@@ -157,35 +206,29 @@ public void notifyUnlock(LlapCacheableBuffer buffer) {
     if (proactiveEvictionEnabled && !instantProactiveEviction) {
       buffer.removeProactiveEvictionMark();
     }
-    int count = threadLocalCount.get();
-    final LlapCacheableBuffer[] cacheableBuffers = threadLocalBuffers.get() ;
-    if (count < maxQueueSize) {
-      cacheableBuffers[count] = buffer;
-      threadLocalCount.set(++count);
-    }
-    if (count <= maxQueueSize / 2) {
-      // case too early to flush
-      return;
-    }
+    BPWrapper bpWrapper = threadLocalBPWrapper.get();
 
-    if (count == maxQueueSize) {
-      // case we have to flush thus block on heap lock
-      heapLock.lock();
-      try {
-        doNotifyUnderHeapLock(count, cacheableBuffers);
-      } finally {
-        threadLocalCount.set(0);
-        heapLock.unlock();
+    // This will only block in a very very rare scenario only.
+    bpWrapper.lock.lock();
+    try {
+      final LlapCacheableBuffer[] cacheableBuffers = bpWrapper.buffers;
+      if (bpWrapper.count < maxQueueSize) {
+        cacheableBuffers[bpWrapper.count] = buffer;
+        ++bpWrapper.count;
       }
-      return;
-    }
-    if (heapLock.tryLock()) {
-      try {
-        doNotifyUnderHeapLock(count, cacheableBuffers);
-      } finally {
-        threadLocalCount.set(0);
-        heapLock.unlock();
+      if (bpWrapper.count <= maxQueueSize / 2) {
+        // case too early to flush
+        return;
+      }
+
+      if (bpWrapper.count == maxQueueSize) {
+        // case we have to flush thus block on heap lock
+        bpWrapper.flush();
+        return;
       }
+      bpWrapper.tryFlush(); //case 50% < queue usage < 100%, flush is preferred but not required yet

Review comment:
       QQ: How did you come up with these numbers? Any tests showing these are the good values?




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



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org