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/12/03 20:52:38 UTC
[1/2] hive git commit: HIVE-12557 : NPE while removing entry in LRFU
cache (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Repository: hive
Updated Branches:
refs/heads/branch-2.0 1946ccbcd -> afd7b9329
refs/heads/master 1d02ab578 -> bef879d0a
HIVE-12557 : NPE while removing entry in LRFU cache (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/bef879d0
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/bef879d0
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/bef879d0
Branch: refs/heads/master
Commit: bef879d0a3e1827bffbd278a883e721124ee0eea
Parents: 1d02ab5
Author: Sergey Shelukhin <se...@apache.org>
Authored: Thu Dec 3 11:52:03 2015 -0800
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Thu Dec 3 11:52:03 2015 -0800
----------------------------------------------------------------------
.../llap/cache/LowLevelLrfuCachePolicy.java | 75 ++++++++++++++++----
1 file changed, 60 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/bef879d0/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 76e7605..40cb92d 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
@@ -140,7 +140,9 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
} else if (heapSize == heap.length) {
// The buffer is not in the (full) heap. Demote the top item of the heap into the list.
LlapCacheableBuffer demoted = heap[0];
- synchronized (listLock) {
+ listLock.lock();
+ try {
+ assert demoted.indexInHeap == 0; // Noone could have moved it, we have the heap lock.
demoted.indexInHeap = LlapCacheableBuffer.IN_LIST;
demoted.prev = null;
if (listHead != null) {
@@ -151,6 +153,8 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
listHead = listTail = demoted;
demoted.next = null;
}
+ } finally {
+ listLock.unlock();
}
// Now insert the buffer in its place and restore heap property.
buffer.indexInHeap = 0;
@@ -340,44 +344,62 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
}
private void removeFromListUnderLock(LlapCacheableBuffer buffer) {
- if (buffer == listTail) {
+ buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
+ boolean isTail = buffer == listTail, isHead = buffer == listHead;
+ if ((isTail != (buffer.next == null)) || (isHead != (buffer.prev == null))) {
+ debugDumpListOnError(buffer);
+ throw new AssertionError("LRFU list is corrupted.");
+ }
+ if (isTail) {
listTail = buffer.prev;
} else {
buffer.next.prev = buffer.prev;
}
- if (buffer == listHead) {
+ if (isHead) {
listHead = buffer.next;
} else {
buffer.prev.next = buffer.next;
}
- buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
}
private void removeFromListUnderLockNoStateUpdate(
LlapCacheableBuffer from, LlapCacheableBuffer to) {
- if (to == listTail) {
+ boolean isToTail = to == listTail, isFromHead = from == listHead;
+ if ((isToTail != (to.next == null)) || (isFromHead != (from.prev == null))) {
+ debugDumpListOnError(from, to);
+ throw new AssertionError("LRFU list is corrupted.");
+ }
+ if (isToTail) {
listTail = from.prev;
} else {
to.next.prev = from.prev;
}
- if (from == listHead) {
+ if (isFromHead) {
listHead = to.next;
} else {
from.prev.next = to.next;
}
}
- public String debugDumpHeap() {
- StringBuilder result = new StringBuilder("List: ");
- if (listHead == null) {
- result.append("<empty>");
- } else {
- LlapCacheableBuffer listItem = listHead;
- while (listItem != null) {
- result.append(listItem.toStringForCache()).append(" -> ");
- listItem = listItem.next;
+ private void debugDumpListOnError(LlapCacheableBuffer... buffers) {
+ // Hopefully this will be helpful in case of NPEs.
+ StringBuilder listDump = new StringBuilder("Invalid list removal. List: ");
+ try {
+ dumpList(listDump, listHead, listTail);
+ int i = 0;
+ for (LlapCacheableBuffer buffer : buffers) {
+ listDump.append("; list from the buffer #").append(i).append(" being removed: ");
+ dumpList(listDump, buffer, null);
}
+ } catch (Throwable t) {
+ LlapIoImpl.LOG.error("Error dumping the lists on error", t);
}
+ LlapIoImpl.LOG.error(listDump.toString());
+ }
+
+ public String debugDumpHeap() {
+ StringBuilder result = new StringBuilder("List: ");
+ dumpList(result, listHead, listTail);
result.append("\nHeap:");
if (heapSize == 0) {
result.append(" <empty>\n");
@@ -421,6 +443,29 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
return result.toString();
}
+ private static void dumpList(StringBuilder result,
+ LlapCacheableBuffer listHeadLocal, LlapCacheableBuffer listTailLocal) {
+ if (listHeadLocal == null) {
+ result.append("<empty>");
+ return;
+ }
+ LlapCacheableBuffer listItem = listHeadLocal;
+ while (listItem.prev != null) {
+ listItem = listItem.prev; // To detect incorrect lists.
+ }
+ while (listItem != null) {
+ result.append(listItem.toStringForCache());
+ if (listItem == listTailLocal) {
+ result.append("(tail)"); // To detect incorrect lists.
+ }
+ if (listItem == listHeadLocal) {
+ result.append("(head)"); // To detect incorrect lists.
+ }
+ result.append(" -> ");
+ listItem = listItem.next;
+ }
+ }
+
@Override
public String debugDumpForOom() {
String result = debugDumpHeap();
[2/2] hive git commit: HIVE-12557 : NPE while removing entry in LRFU
cache (Sergey Shelukhin, reviewed by Prasanth Jayachandran)
Posted by se...@apache.org.
HIVE-12557 : NPE while removing entry in LRFU cache (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/afd7b932
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/afd7b932
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/afd7b932
Branch: refs/heads/branch-2.0
Commit: afd7b932985d93b9d287f7104a5d0277e12d3b14
Parents: 1946ccb
Author: Sergey Shelukhin <se...@apache.org>
Authored: Thu Dec 3 11:52:03 2015 -0800
Committer: Sergey Shelukhin <se...@apache.org>
Committed: Thu Dec 3 11:52:13 2015 -0800
----------------------------------------------------------------------
.../llap/cache/LowLevelLrfuCachePolicy.java | 75 ++++++++++++++++----
1 file changed, 60 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/afd7b932/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 76e7605..40cb92d 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
@@ -140,7 +140,9 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
} else if (heapSize == heap.length) {
// The buffer is not in the (full) heap. Demote the top item of the heap into the list.
LlapCacheableBuffer demoted = heap[0];
- synchronized (listLock) {
+ listLock.lock();
+ try {
+ assert demoted.indexInHeap == 0; // Noone could have moved it, we have the heap lock.
demoted.indexInHeap = LlapCacheableBuffer.IN_LIST;
demoted.prev = null;
if (listHead != null) {
@@ -151,6 +153,8 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
listHead = listTail = demoted;
demoted.next = null;
}
+ } finally {
+ listLock.unlock();
}
// Now insert the buffer in its place and restore heap property.
buffer.indexInHeap = 0;
@@ -340,44 +344,62 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
}
private void removeFromListUnderLock(LlapCacheableBuffer buffer) {
- if (buffer == listTail) {
+ buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
+ boolean isTail = buffer == listTail, isHead = buffer == listHead;
+ if ((isTail != (buffer.next == null)) || (isHead != (buffer.prev == null))) {
+ debugDumpListOnError(buffer);
+ throw new AssertionError("LRFU list is corrupted.");
+ }
+ if (isTail) {
listTail = buffer.prev;
} else {
buffer.next.prev = buffer.prev;
}
- if (buffer == listHead) {
+ if (isHead) {
listHead = buffer.next;
} else {
buffer.prev.next = buffer.next;
}
- buffer.indexInHeap = LlapCacheableBuffer.NOT_IN_CACHE;
}
private void removeFromListUnderLockNoStateUpdate(
LlapCacheableBuffer from, LlapCacheableBuffer to) {
- if (to == listTail) {
+ boolean isToTail = to == listTail, isFromHead = from == listHead;
+ if ((isToTail != (to.next == null)) || (isFromHead != (from.prev == null))) {
+ debugDumpListOnError(from, to);
+ throw new AssertionError("LRFU list is corrupted.");
+ }
+ if (isToTail) {
listTail = from.prev;
} else {
to.next.prev = from.prev;
}
- if (from == listHead) {
+ if (isFromHead) {
listHead = to.next;
} else {
from.prev.next = to.next;
}
}
- public String debugDumpHeap() {
- StringBuilder result = new StringBuilder("List: ");
- if (listHead == null) {
- result.append("<empty>");
- } else {
- LlapCacheableBuffer listItem = listHead;
- while (listItem != null) {
- result.append(listItem.toStringForCache()).append(" -> ");
- listItem = listItem.next;
+ private void debugDumpListOnError(LlapCacheableBuffer... buffers) {
+ // Hopefully this will be helpful in case of NPEs.
+ StringBuilder listDump = new StringBuilder("Invalid list removal. List: ");
+ try {
+ dumpList(listDump, listHead, listTail);
+ int i = 0;
+ for (LlapCacheableBuffer buffer : buffers) {
+ listDump.append("; list from the buffer #").append(i).append(" being removed: ");
+ dumpList(listDump, buffer, null);
}
+ } catch (Throwable t) {
+ LlapIoImpl.LOG.error("Error dumping the lists on error", t);
}
+ LlapIoImpl.LOG.error(listDump.toString());
+ }
+
+ public String debugDumpHeap() {
+ StringBuilder result = new StringBuilder("List: ");
+ dumpList(result, listHead, listTail);
result.append("\nHeap:");
if (heapSize == 0) {
result.append(" <empty>\n");
@@ -421,6 +443,29 @@ public class LowLevelLrfuCachePolicy implements LowLevelCachePolicy {
return result.toString();
}
+ private static void dumpList(StringBuilder result,
+ LlapCacheableBuffer listHeadLocal, LlapCacheableBuffer listTailLocal) {
+ if (listHeadLocal == null) {
+ result.append("<empty>");
+ return;
+ }
+ LlapCacheableBuffer listItem = listHeadLocal;
+ while (listItem.prev != null) {
+ listItem = listItem.prev; // To detect incorrect lists.
+ }
+ while (listItem != null) {
+ result.append(listItem.toStringForCache());
+ if (listItem == listTailLocal) {
+ result.append("(tail)"); // To detect incorrect lists.
+ }
+ if (listItem == listHeadLocal) {
+ result.append("(head)"); // To detect incorrect lists.
+ }
+ result.append(" -> ");
+ listItem = listItem.next;
+ }
+ }
+
@Override
public String debugDumpForOom() {
String result = debugDumpHeap();