You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2015/10/23 23:12:32 UTC
hive git commit: HIVE-12178 : LLAP: NPE in LRFU policy (Sergey
Shelukhin, reviewed by Prasanth Jayachandran)
Repository: hive
Updated Branches:
refs/heads/master 6d4aa0686 -> e9cdea925
HIVE-12178 : LLAP: NPE in LRFU policy (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/e9cdea92
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/e9cdea92
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/e9cdea92
Branch: refs/heads/master
Commit: e9cdea92524c6d710f3d81e2bb982a6303e041bf
Parents: 6d4aa06
Author: Sergey Shelukhin <se...@apache.org>
Authored: Fri Oct 23 14:02:11 2015 -0700
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Fri Oct 23 14:02:11 2015 -0700
----------------------------------------------------------------------
.../org/apache/hadoop/hive/conf/HiveConf.java | 3 +-
.../llap/cache/LowLevelLrfuCachePolicy.java | 8 +--
.../llap/cache/TestLowLevelLrfuCachePolicy.java | 57 +++++++++++++++++---
3 files changed, 57 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/e9cdea92/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
index 23ae0dc..f065048 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
@@ -2305,7 +2305,8 @@ public class HiveConf extends Configuration {
LLAP_USE_LRFU("hive.llap.io.use.lrfu", false,
"Whether ORC low-level cache should use LRFU cache policy instead of default (FIFO)."),
LLAP_LRFU_LAMBDA("hive.llap.io.lrfu.lambda", 0.01f,
- "Lambda for ORC low-level cache LRFU cache policy."),
+ "Lambda for ORC low-level cache LRFU cache policy. Must be in [0, 1]. 0 makes LRFU\n" +
+ "behave like LFU, 1 makes it behave like LRU, values in between balance accordingly."),
LLAP_ORC_ENABLE_TIME_COUNTERS("hive.llap.io.orc.time.counters", true,
"Whether to enable time counters for LLAP IO layer (time spent in HDFS, etc.)"),
LLAP_AUTO_ALLOW_UBER("hive.llap.auto.allow.uber", true,
http://git-wip-us.apache.org/repos/asf/hive/blob/e9cdea92/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java
----------------------------------------------------------------------
diff --git a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java
index f551edb..76e7605 100644
--- a/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java
+++ b/llap-server/src/java/org/apache/hadoop/hive/llap/cache/LowLevelLrfuCachePolicy.java
@@ -205,7 +205,8 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
listHead = listTail = null; // We have evicted the entire list.
} else {
// Splice the section that we have evicted out of the list.
- removeFromListUnderLock(nextCandidate.next, firstCandidate);
+ // We have already updated the state above so no need to do that again.
+ removeFromListUnderLockNoStateUpdate(nextCandidate.next, firstCandidate);
}
}
} finally {
@@ -333,7 +334,6 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
try {
if (buffer.indexInHeap != LlapCacheableBuffer.IN_LIST) return;
removeFromListUnderLock(buffer);
- buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
} finally {
listLock.unlock();
}
@@ -350,9 +350,11 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
} else {
buffer.prev.next = buffer.next;
}
+ buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
}
- private void removeFromListUnderLock(LlapCacheableBuffer from, LlapCacheableBuffer to) {
+ private void removeFromListUnderLockNoStateUpdate(
+ LlapCacheableBuffer from, LlapCacheableBuffer to) {
if (to == listTail) {
listTail = from.prev;
} else {
http://git-wip-us.apache.org/repos/asf/hive/blob/e9cdea92/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java
----------------------------------------------------------------------
diff --git a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java
index a423eeb..bb815e3 100644
--- a/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java
+++ b/llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestLowLevelLrfuCachePolicy.java
@@ -17,17 +17,14 @@
*/
package org.apache.hadoop.hive.llap.cache;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Random;
+import java.util.concurrent.locks.ReentrantLock;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -42,6 +39,45 @@ public class TestLowLevelLrfuCachePolicy {
private static final Log LOG = LogFactory.getLog(TestLowLevelLrfuCachePolicy.class);
@Test
+ public void testRegression_HIVE_12178() throws Exception {
+ LOG.info("Testing wrong list status after eviction");
+ EvictionTracker et = new EvictionTracker();
+ int memSize = 2, lambda = 1; // Set lambda to 1 so the heap size becomes 1 (LRU).
+ Configuration conf = createConf(1, memSize, (double)lambda);
+ final LowLevelLrfuCachePolicy lrfu = new LowLevelLrfuCachePolicy(conf);
+ Field f = LowLevelLrfuCachePolicy.class.getDeclaredField("listLock");
+ f.setAccessible(true);
+ ReentrantLock listLock = (ReentrantLock)f.get(lrfu);
+ LowLevelCacheMemoryManager mm = new LowLevelCacheMemoryManager(conf, lrfu,
+ LlapDaemonCacheMetrics.create("test", "1"));
+ lrfu.setEvictionListener(et);
+ final LlapDataBuffer buffer1 = LowLevelCacheImpl.allocateFake();
+ LlapDataBuffer buffer2 = LowLevelCacheImpl.allocateFake();
+ assertTrue(cache(mm, lrfu, et, buffer1));
+ assertTrue(cache(mm, lrfu, et, buffer2));
+ // buffer2 is now in the heap, buffer1 is in the list. "Use" buffer1 again;
+ // before we notify though, lock the list, so lock cannot remove it from the list.
+ buffer1.incRef();
+ assertEquals(LlapCacheableBuffer.IN_LIST, buffer1.indexInHeap);
+ listLock.lock();
+ try {
+ Thread otherThread = new Thread(new Runnable() {
+ public void run() {
+ lrfu.notifyLock(buffer1);
+ }
+ });
+ otherThread.start();
+ otherThread.join();
+ } finally {
+ listLock.unlock();
+ }
+ // Now try to evict with locked buffer still in the list.
+ mm.reserveMemory(1, false);
+ assertSame(buffer2, et.evicted.get(0));
+ unlock(lrfu, buffer1);
+ }
+
+ @Test
public void testHeapSize2() {
testHeapSize(2);
}
@@ -100,13 +136,20 @@ public class TestLowLevelLrfuCachePolicy {
verifyOrder(mm, lfu, et, inserted);
}
- private Configuration createConf(int min, int heapSize) {
+ private Configuration createConf(int min, int heapSize, Double lambda) {
Configuration conf = new Configuration();
conf.setInt(HiveConf.ConfVars.LLAP_ORC_CACHE_MIN_ALLOC.varname, min);
conf.setInt(HiveConf.ConfVars.LLAP_ORC_CACHE_MAX_SIZE.varname, heapSize);
+ if (lambda != null) {
+ conf.setDouble(HiveConf.ConfVars.LLAP_LRFU_LAMBDA.varname, lambda.doubleValue());
+ }
return conf;
}
+ private Configuration createConf(int min, int heapSize) {
+ return createConf(min, heapSize, null);
+ }
+
@Test
public void testLruExtreme() {
int heapSize = 4;