Michael Blow has submitted this change and it was merged.

Change subject: [ASTERIXDB-2270][STO] Fix Deadlock in LSMRTree
......................................................................


[ASTERIXDB-2270][STO] Fix Deadlock in LSMRTree

- user model changes: no
- storage format changes: no
- interface changes: no

details:
- The LSMRTreeWithAntiMatterTuplesSearchCursor was
  using the includeMutableComponent incorrectly to
  determine whether memory components are included.
- This change ensures that memory components cursors
  are always closed and destroyed as part of the
  cursor lifecycle.
- The change also includes a check that fails if a
  virtual file was deleted and some of its pages
  are still latched.

Validate pages of deleted vbc files are not latched

Change-Id: If91ac3a2fa9265f929867e0dc75bbe0b7fa0aed1
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2343
Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Reviewed-by: Michael Blow <mb...@apache.org>
---
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
M 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
M 
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/VirtualPage.java
3 files changed, 45 insertions(+), 26 deletions(-)

Approvals:
  Anon. E. Moose #1000171: 
  Jenkins: Verified; No violations found; 
  Michael Blow: Looks good to me, approved



diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
index d192351..1cfe345 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
@@ -46,7 +46,7 @@
 
 public class VirtualBufferCache implements IVirtualBufferCache {
     private static final Logger LOGGER = LogManager.getLogger();
-
+    private static final boolean DEBUG = true;
     private final ICacheMemoryAllocator allocator;
     private final IFileMapManager fileMapManager;
     private final int pageSize;
@@ -198,6 +198,13 @@
         // recycle only if
         // 1. not a large page
         // 2. allocation is not above budget
+        if (DEBUG) {
+            int readCount = page.getReadLatchCount();
+            if (readCount > 0 || page.isWriteLatched()) {
+                throw new IllegalStateException("Attempt to delete a file with 
latched pages (read: " + readCount
+                        + ", write: " + page.isWriteLatched() + ")");
+            }
+        }
         if (used.get() < pageBudget && !page.isLargePage()) {
             page.reset();
             freePages.offer(page);
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
index 7ae72a9..b8180a6 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-rtree/src/main/java/org/apache/hyracks/storage/am/lsm/rtree/impls/LSMRTreeWithAntiMatterTuplesSearchCursor.java
@@ -56,7 +56,7 @@
     private MultiComparator btreeCmp;
     private int currentCursor;
     private SearchPredicate rtreeSearchPredicate;
-    private int numMutableComponents;
+    private int numMemoryComponents;
     private boolean open;
     protected ISearchOperationCallback searchCallback;
 
@@ -79,27 +79,24 @@
         operationalComponents = lsmInitialState.getOperationalComponents();
         rtreeSearchPredicate = (SearchPredicate) searchPred;
         searchCallback = lsmInitialState.getSearchOperationCallback();
-
-        includeMutableComponent = false;
-        numMutableComponents = 0;
+        numMemoryComponents = 0;
         int numImmutableComponents = 0;
         for (ILSMComponent component : operationalComponents) {
             if (component.getType() == LSMComponentType.MEMORY) {
-                includeMutableComponent = true;
-                numMutableComponents++;
+                numMemoryComponents++;
             } else {
                 numImmutableComponents++;
             }
         }
-        if (includeMutableComponent) {
+        if (numMemoryComponents > 0) {
             btreeRangePredicate = new RangePredicate(null, null, true, true, 
btreeCmp, btreeCmp);
         }
 
-        mutableRTreeCursors = new RTreeSearchCursor[numMutableComponents];
-        mutableRTreeAccessors = new ITreeIndexAccessor[numMutableComponents];
-        btreeCursors = new BTreeRangeSearchCursor[numMutableComponents];
-        btreeAccessors = new ITreeIndexAccessor[numMutableComponents];
-        for (int i = 0; i < numMutableComponents; i++) {
+        mutableRTreeCursors = new RTreeSearchCursor[numMemoryComponents];
+        mutableRTreeAccessors = new ITreeIndexAccessor[numMemoryComponents];
+        btreeCursors = new BTreeRangeSearchCursor[numMemoryComponents];
+        btreeAccessors = new ITreeIndexAccessor[numMemoryComponents];
+        for (int i = 0; i < numMemoryComponents; i++) {
             ILSMComponent component = operationalComponents.get(i);
             RTree rtree = ((LSMRTreeMemoryComponent) component).getIndex();
             BTree btree = ((LSMRTreeMemoryComponent) 
component).getBuddyIndex();
@@ -115,7 +112,7 @@
         rangeCursors = new RTreeSearchCursor[numImmutableComponents];
         ITreeIndexAccessor[] immutableRTreeAccessors = new 
ITreeIndexAccessor[numImmutableComponents];
         int j = 0;
-        for (int i = numMutableComponents; i < operationalComponents.size(); 
i++) {
+        for (int i = numMemoryComponents; i < operationalComponents.size(); 
i++) {
             ILSMComponent component = operationalComponents.get(i);
             rangeCursors[j] = new RTreeSearchCursor(
                     (IRTreeInteriorFrame) 
lsmInitialState.getRTreeInteriorFrameFactory().createFrame(),
@@ -132,7 +129,7 @@
     }
 
     private void searchNextCursor() throws HyracksDataException {
-        if (currentCursor < numMutableComponents) {
+        if (currentCursor < numMemoryComponents) {
             mutableRTreeCursors[currentCursor].close();
             
mutableRTreeAccessors[currentCursor].search(mutableRTreeCursors[currentCursor], 
rtreeSearchPredicate);
         }
@@ -140,12 +137,12 @@
 
     @Override
     public boolean hasNext() throws HyracksDataException {
-        if (includeMutableComponent) {
+        if (numMemoryComponents > 0) {
             if (foundNext) {
                 return true;
             }
 
-            while (currentCursor < numMutableComponents) {
+            while (currentCursor < numMemoryComponents) {
                 while (mutableRTreeCursors[currentCursor].hasNext()) {
                     mutableRTreeCursors[currentCursor].next();
                     ITupleReference currentTuple = 
mutableRTreeCursors[currentCursor].getTuple();
@@ -173,7 +170,7 @@
 
                 // Call proceed() to do necessary operations before returning 
this tuple.
                 searchCallback.proceed(diskRTreeTuple);
-                if (searchMemBTrees(diskRTreeTuple, numMutableComponents)) {
+                if (searchMemBTrees(diskRTreeTuple, numMemoryComponents)) {
                     // anti-matter tuple is NOT found
                     foundNext = true;
                     frameTuple = diskRTreeTuple;
@@ -203,7 +200,7 @@
     @Override
     public ITupleReference getFilterMinTuple() {
         ILSMComponentFilter filter = operationalComponents.get(
-                currentCursor < numMutableComponents ? currentCursor : 
outputElement.getCursorIndex() + currentCursor)
+                currentCursor < numMemoryComponents ? currentCursor : 
outputElement.getCursorIndex() + currentCursor)
                 .getLSMComponentFilter();
         return filter == null ? null : filter.getMinTuple();
     }
@@ -211,7 +208,7 @@
     @Override
     public ITupleReference getFilterMaxTuple() {
         ILSMComponentFilter filter = operationalComponents.get(
-                currentCursor < numMutableComponents ? currentCursor : 
outputElement.getCursorIndex() + currentCursor)
+                currentCursor < numMemoryComponents ? currentCursor : 
outputElement.getCursorIndex() + currentCursor)
                 .getLSMComponentFilter();
         return filter == null ? null : filter.getMaxTuple();
     }
@@ -233,8 +230,8 @@
         }
         currentCursor = 0;
         foundNext = false;
-        if (includeMutableComponent) {
-            for (int i = 0; i < numMutableComponents; i++) {
+        if (numMemoryComponents > 0) {
+            for (int i = 0; i < numMemoryComponents; i++) {
                 mutableRTreeCursors[i].close();
                 btreeCursors[i].close();
             }
@@ -247,8 +244,8 @@
         if (!open) {
             return;
         }
-        if (includeMutableComponent) {
-            for (int i = 0; i < numMutableComponents; i++) {
+        if (numMemoryComponents > 0) {
+            for (int i = 0; i < numMemoryComponents; i++) {
                 mutableRTreeCursors[i].destroy();
                 btreeCursors[i].destroy();
             }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/VirtualPage.java
 
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/VirtualPage.java
index 139a3c4..be384e0 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/VirtualPage.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/VirtualPage.java
@@ -19,11 +19,10 @@
 package org.apache.hyracks.storage.common.buffercache;
 
 import java.nio.ByteBuffer;
-import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 public class VirtualPage implements ICachedPage {
-    private final ReadWriteLock latch;
+    private final ReentrantReadWriteLock latch;
     private final int pageSize;
     private ByteBuffer buffer;
     private volatile long dpid;
@@ -131,4 +130,20 @@
         return multiplier > 1;
     }
 
+    public int getReadLatchCount() {
+        return latch.getReadLockCount();
+    }
+
+    public boolean isWriteLatched() {
+        return latch.isWriteLocked();
+    }
+
+    @Override
+    public String toString() {
+        StringBuilder str = new StringBuilder();
+        str.append("{\"class\":\"" + getClass().getSimpleName() + "\", 
\"readers\":" + getReadLatchCount()
+                + ",\"writers\":" + (isWriteLatched()));
+        str.append("}");
+        return str.toString();
+    }
 }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2343
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: If91ac3a2fa9265f929867e0dc75bbe0b7fa0aed1
Gerrit-PatchSet: 9
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Luo Chen <cl...@uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>

Reply via email to