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>
