hive-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ser...@apache.org
Subject hive git commit: HIVE-16278 : LLAP: metadata cache may incorrectly decrease memory usage in mem manager (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Date Thu, 23 Mar 2017 19:00:49 GMT
Repository: hive
Updated Branches:
  refs/heads/master 4812486a1 -> 0466fca73


HIVE-16278 : LLAP: metadata cache may incorrectly decrease memory usage in mem manager (Sergey
Shelukhin, reviewed by Prasanth Jayachandran)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/0466fca7
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/0466fca7
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/0466fca7

Branch: refs/heads/master
Commit: 0466fca73282969dc25e8cde2f37e481964ebe59
Parents: 4812486
Author: Sergey Shelukhin <sershe@apache.org>
Authored: Thu Mar 23 11:55:00 2017 -0700
Committer: Sergey Shelukhin <sershe@apache.org>
Committed: Thu Mar 23 11:55:00 2017 -0700

----------------------------------------------------------------------
 .../hadoop/hive/llap/cache/BuddyAllocator.java  | 34 ++++++++++++++++----
 .../llap/cache/LowLevelCacheMemoryManager.java  | 14 ++++++++
 .../hadoop/hive/llap/cache/MemoryManager.java   |  2 +-
 .../hive/llap/cache/SerDeLowLevelCacheImpl.java | 27 +---------------
 .../hive/llap/io/metadata/OrcMetadataCache.java |  7 ++--
 .../hive/llap/cache/TestBuddyAllocator.java     |  3 +-
 .../hive/llap/cache/TestOrcMetadataCache.java   |  3 +-
 .../ql/io/orc/encoded/EncodedReaderImpl.java    |  4 +--
 8 files changed, 51 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java
index f1ae5b4..e71a09e 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/BuddyAllocator.java
@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hive.llap.cache;
 
+import java.util.concurrent.atomic.AtomicLong;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
@@ -48,6 +50,8 @@ public final class BuddyAllocator implements EvictionAwareAllocator, BuddyAlloca
   private final AtomicInteger allocatedArenas = new AtomicInteger(0);
 
   private final MemoryManager memoryManager;
+  private static final long MAX_DUMP_INTERVAL_NS = 300 * 1000000000L; // 5 minutes.
+  private final AtomicLong lastLog = new AtomicLong(-1);
 
   // Config settings
   private final int minAllocLog2, maxAllocLog2, arenaSizeLog2, maxArenas;
@@ -191,8 +195,7 @@ public final class BuddyAllocator implements EvictionAwareAllocator, BuddyAlloca
     int allocLog2 = freeListIx + minAllocLog2;
     int allocationSize = 1 << allocLog2;
     // TODO: reserving the entire thing is not ideal before we alloc anything. Interleave?
-    memoryManager.reserveMemory(dest.length << allocLog2, true);
-
+    memoryManager.reserveMemory(dest.length << allocLog2);
     int destAllocIx = 0;
     for (int i = 0; i < dest.length; ++i) {
       if (dest[i] != null) continue;
@@ -269,23 +272,23 @@ public final class BuddyAllocator implements EvictionAwareAllocator,
BuddyAlloca
             if (destAllocIx == dest.length) return;
           }
         }
-        int numberToForce = (dest.length - destAllocIx) * attempt;
+        int numberToForce = (dest.length - destAllocIx) * (attempt + 1);
         long newReserved = memoryManager.forceReservedMemory(allocationSize, numberToForce);
         forceReserved += newReserved;
         if (newReserved == 0) {
           // Cannot force-evict anything, give up.
           String msg = "Failed to allocate " + size + "; at " + destAllocIx + " out of "
               + dest.length + " (entire cache is fragmented and locked, or an internal issue)";
-          LlapIoImpl.LOG.error(msg + "\nALLOCATOR STATE:\n" + debugDump()
-              + "\nPARENT STATE:\n" + memoryManager.debugDumpForOom());
+          logOomErrorMessage(msg);
           throw new AllocatorOutOfMemoryException(msg);
         }
         if (attempt == 0) {
           LlapIoImpl.LOG.warn("Failed to allocate despite reserved memory; will retry");
         }
+        ++attempt;
       }
     } finally {
-      if (attempt > 1) {
+      if (attempt > 4) {
         LlapIoImpl.LOG.warn("Allocation of " + dest.length + " buffers of size " + size
             + " took " + attempt + " attempts to evict enough memory");
       }
@@ -299,6 +302,25 @@ public final class BuddyAllocator implements EvictionAwareAllocator,
BuddyAlloca
 
   }
 
+  private void logOomErrorMessage(String msg) {
+    while (true) {
+      long time = System.nanoTime();
+      long lastTime = lastLog.get();
+      // Magic value usage is invalid with nanoTime, so once in a 1000 years we may log extra.
+      boolean shouldLog = (lastTime == -1 || (time - lastTime) > MAX_DUMP_INTERVAL_NS);
+      if (shouldLog && !lastLog.compareAndSet(lastTime, time)) {
+        continue;
+      }
+      if (shouldLog) {
+        LlapIoImpl.LOG.error(msg + "\nALLOCATOR STATE:\n" + debugDump()
+            + "\nPARENT STATE:\n" + memoryManager.debugDumpForOom());
+      } else {
+        LlapIoImpl.LOG.error(msg);
+      }
+      return;
+    }
+  }
+
   @Override
   public void deallocate(MemoryBuffer buffer) {
     deallocateInternal(buffer, true);

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java
b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java
index 8e10fd4..5232d8c 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelCacheMemoryManager.java
@@ -56,7 +56,17 @@ public class LowLevelCacheMemoryManager implements MemoryManager {
     }
   }
 
+
   @Override
+  public void reserveMemory(final long memoryToReserve) {
+    boolean result = reserveMemory(memoryToReserve, true);
+    if (result) return;
+    // Can only happen if there's no evictor, or if thread is interrupted.
+    throw new RuntimeException("Cannot reserve memory"
+        + (Thread.currentThread().isInterrupted() ? "; thread interrupted" : ""));
+  }
+
+  @VisibleForTesting
   public boolean reserveMemory(final long memoryToReserve, boolean waitForEviction) {
     // TODO: if this cannot evict enough, it will spin infinitely. Terminate at some point?
     int badCallCount = 0;
@@ -108,6 +118,10 @@ public class LowLevelCacheMemoryManager implements MemoryManager {
         usedMem = usedMemory.get();
       }
     }
+    if (!result) {
+      releaseMemory(reservedTotalMetric);
+      reservedTotalMetric = 0;
+    }
     metrics.incrCacheCapacityUsed(reservedTotalMetric - evictedTotalMetric);
     return result;
   }

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java
index 05e901f..0f4d3c0 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/MemoryManager.java
@@ -19,9 +19,9 @@
 package org.apache.hadoop.hive.llap.cache;
 
 public interface MemoryManager extends LlapOomDebugDump {
-  boolean reserveMemory(long memoryToReserve, boolean waitForEviction);
   void releaseMemory(long memUsage);
   void updateMaxSize(long maxSize);
   /** TODO: temporary method until we get a better allocator. */
   long forceReservedMemory(int allocationSize, int count);
+  void reserveMemory(long memoryToReserve);
 }

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/java/org/apache/hadoop/hive/llap/cache/SerDeLowLevelCacheImpl.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/SerDeLowLevelCacheImpl.java
b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/SerDeLowLevelCacheImpl.java
index 4809398..917c4a3 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/SerDeLowLevelCacheImpl.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/SerDeLowLevelCacheImpl.java
@@ -23,7 +23,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -44,7 +43,7 @@ import org.apache.orc.OrcProto.ColumnEncoding;
 
 import com.google.common.base.Function;
 
-public class SerDeLowLevelCacheImpl implements BufferUsageManager, LlapOomDebugDump {
+public class SerDeLowLevelCacheImpl implements LlapOomDebugDump {
   private static final int DEFAULT_CLEANUP_INTERVAL = 600;
   private final Allocator allocator;
   private final AtomicInteger newEvictions = new AtomicInteger(0);
@@ -617,18 +616,6 @@ public class SerDeLowLevelCacheImpl implements BufferUsageManager, LlapOomDebugD
     } 
   }
 
-  @Override
-  public void decRefBuffer(MemoryBuffer buffer) {
-    unlockBuffer((LlapDataBuffer)buffer, true);
-  }
-
-  @Override
-  public void decRefBuffers(List<MemoryBuffer> cacheBuffers) {
-    for (MemoryBuffer b : cacheBuffers) {
-      unlockBuffer((LlapDataBuffer)b, true);
-    }
-  }
-
   private void unlockBuffer(LlapDataBuffer buffer, boolean handleLastDecRef) {
     boolean isLastDecref = (buffer.decRef() == 0);
     if (handleLastDecRef && isLastDecref) {
@@ -704,18 +691,6 @@ public class SerDeLowLevelCacheImpl implements BufferUsageManager, LlapOomDebugD
   }
 
   @Override
-  public boolean incRefBuffer(MemoryBuffer buffer) {
-    // notifyReused implies that buffer is already locked; it's also called once for new
-    // buffers that are not cached yet. Don't notify cache policy.
-    return lockBuffer(((LlapDataBuffer)buffer), false);
-  }
-
-  @Override
-  public Allocator getAllocator() {
-    return allocator;
-  }
-
-  @Override
   public String debugDumpForOom() {
     StringBuilder sb = new StringBuilder("File cache state ");
     for (Map.Entry<Object, FileCache<FileData>> e : cache.entrySet()) {

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcMetadataCache.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcMetadataCache.java
b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcMetadataCache.java
index 73a1721..2645428 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcMetadataCache.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcMetadataCache.java
@@ -31,7 +31,6 @@ import org.apache.hadoop.hive.llap.cache.LowLevelCachePolicy;
 import org.apache.hadoop.hive.llap.cache.MemoryManager;
 import org.apache.hadoop.hive.llap.cache.LowLevelCache.Priority;
 import org.apache.hadoop.hive.ql.io.orc.encoded.OrcBatchKey;
-import org.apache.hadoop.hive.ql.util.JavaDataModel;
 
 public class OrcMetadataCache {
   private final ConcurrentHashMap<Object, OrcFileMetadata> metadata = new ConcurrentHashMap<>();
@@ -51,7 +50,7 @@ public class OrcMetadataCache {
 
   public OrcFileMetadata putFileMetadata(OrcFileMetadata metaData) {
     long memUsage = metaData.getMemoryUsage();
-    memoryManager.reserveMemory(memUsage, false);
+    memoryManager.reserveMemory(memUsage);
     OrcFileMetadata val = metadata.putIfAbsent(metaData.getFileKey(), metaData);
     // See OrcFileMetadata; it is always unlocked, so we just "touch" it here to simulate
use.
     return touchOnPut(metaData, val, memUsage);
@@ -59,7 +58,7 @@ public class OrcMetadataCache {
 
   public OrcStripeMetadata putStripeMetadata(OrcStripeMetadata metaData) {
     long memUsage = metaData.getMemoryUsage();
-    memoryManager.reserveMemory(memUsage, false);
+    memoryManager.reserveMemory(memUsage);
     OrcStripeMetadata val = stripeMetadata.putIfAbsent(metaData.getKey(), metaData);
     // See OrcStripeMetadata; it is always unlocked, so we just "touch" it here to simulate
use.
     return touchOnPut(metaData, val, memUsage);
@@ -90,7 +89,7 @@ public class OrcMetadataCache {
         errorData.addError(range.getOffset(), range.getLength(), baseOffset);
       }
       long memUsage = errorData.estimateMemoryUsage();
-      memoryManager.reserveMemory(memUsage, false);
+      memoryManager.reserveMemory(memUsage);
       OrcFileEstimateErrors old = estimateErrors.putIfAbsent(fileKey, errorData);
       if (old != null) {
         errorData = old;

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java
----------------------------------------------------------------------
diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java
b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java
index 3a8ca2e..f621005 100644
--- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java
+++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestBuddyAllocator.java
@@ -58,8 +58,7 @@ public class TestBuddyAllocator {
 
   private static class DummyMemoryManager implements MemoryManager {
     @Override
-    public boolean reserveMemory(long memoryToReserve, boolean waitForEviction) {
-      return true;
+    public void reserveMemory(long memoryToReserve) {
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java
----------------------------------------------------------------------
diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java
b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java
index be1be7a..4855ed7 100644
--- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java
+++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestOrcMetadataCache.java
@@ -72,9 +72,8 @@ public class TestOrcMetadataCache {
     int allocs = 0;
 
     @Override
-    public boolean reserveMemory(long memoryToReserve, boolean waitForEviction) {
+    public void reserveMemory(long memoryToReserve) {
       ++allocs;
-      return true;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/hive/blob/0466fca7/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java
index 19f3451..326b4b6 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/io/orc/encoded/EncodedReaderImpl.java
@@ -220,8 +220,8 @@ class EncodedReaderImpl implements EncodedReader {
 
   @Override
   public void readEncodedColumns(int stripeIx, StripeInformation stripe,
-      OrcProto.RowIndex[] indexes, List<OrcProto.ColumnEncoding> encodings, List<OrcProto.Stream>
streamList,
-      boolean[] included, boolean[][] colRgs,
+      OrcProto.RowIndex[] indexes, List<OrcProto.ColumnEncoding> encodings,
+      List<OrcProto.Stream> streamList, boolean[] included, boolean[][] colRgs,
       Consumer<OrcEncodedColumnBatch> consumer) throws IOException {
     // Note: for now we don't have to setError here, caller will setError if we throw.
     // We are also not supposed to call setDone, since we are only part of the operation.


Mime
View raw message