[NO ISSUE][HYR][STO] BufferCache lock fixes
Change-Id: I37f06163bbf1c34392d83a8ccd27e777552eeac7
Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18181
Reviewed-by: Michael Blow <mblow@apache.org>
Tested-by: Michael Blow <mblow@apache.org>
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java
index 78faaff..e085b5e 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java
@@ -105,7 +105,7 @@
RangePredicate diskOrderScanPred = new RangePredicate(null, null, true, true, ctx.getCmp(), ctx.getCmp());
int maxPageId = freePageManager.getMaxPageId(ctx.getMetaFrame());
int currentPageId = bulkloadLeafStart;
- ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), currentPageId), false);
+ final ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), currentPageId), false);
page.acquireReadLatch();
try {
cursor.setBufferCache(bufferCache);
@@ -116,7 +116,7 @@
ctx.getCursorInitialState().setSearchOperationCallback(ctx.getSearchCallback());
ctx.getCursorInitialState().setOriginialKeyComparator(ctx.getCmp());
cursor.open(ctx.getCursorInitialState(), diskOrderScanPred);
- } catch (Exception e) {
+ } catch (Throwable e) {
page.releaseReadLatch();
bufferCache.unpin(page);
throw HyracksDataException.create(e);
@@ -202,8 +202,7 @@
}
// we use this loop to deal with possibly multiple operation restarts
// due to ongoing structure modifications during the descent
- boolean repeatOp = true;
- while (repeatOp && ctx.getOpRestarts() < MAX_RESTARTS) {
+ while (true) {
performOp(rootPage, null, true, ctx);
// if we reach this stage then we need to restart from the (possibly
// new) root
@@ -211,7 +210,7 @@
ctx.getPageLsns().removeLast(); // pop the restart op indicator
continue;
}
- repeatOp = false;
+ break;
}
cursor.setBufferCache(bufferCache);
cursor.setFileId(getFileId());
@@ -231,12 +230,8 @@
bufferCache.unpin(smPage);
}
}
- if (ctx.getSmPages().size() > 0) {
- if (ctx.getSmoCount() == Integer.MAX_VALUE) {
- smoCounter.set(0);
- } else {
- smoCounter.incrementAndGet();
- }
+ if (!ctx.getSmPages().isEmpty()) {
+ smoCounter.updateAndGet(i -> i == Integer.MAX_VALUE ? 0 : i + 1);
treeLatch.writeLock().unlock();
ctx.getSmPages().clear();
}
@@ -599,8 +594,7 @@
// We use this loop to deal with possibly multiple operation
// restarts due to ongoing structure modifications during
// the descent.
- boolean repeatOp = true;
- while (repeatOp && ctx.getOpRestarts() < MAX_RESTARTS) {
+ while (true) {
int childPageId = ctx.getInteriorFrame().getChildPageId(ctx.getPred());
performOp(childPageId, node, isReadLatched, ctx);
node = null;
@@ -615,6 +609,10 @@
if (node != null) {
isReadLatched = true;
// Descend the tree again.
+ if (ctx.getOpRestarts() >= MAX_RESTARTS) {
+ throw HyracksDataException.create(ErrorCode.OPERATION_EXCEEDED_MAX_RESTARTS,
+ MAX_RESTARTS);
+ }
continue;
} else {
// Pop pageLsn of this page (version seen by this op during descent).
@@ -662,7 +660,7 @@
}
}
// Operation completed.
- repeatOp = false;
+ break;
} // end while
} else { // smFlag
ctx.setOpRestarts(ctx.getOpRestarts() + 1);
@@ -734,21 +732,8 @@
ctx.getPageLsns().add(FULL_RESTART_OP);
}
}
- } catch (HyracksDataException e) {
- if (!ctx.isExceptionHandled()) {
- if (node != null) {
- if (isReadLatched) {
- node.releaseReadLatch();
- } else {
- node.releaseWriteLatch(true);
- }
- bufferCache.unpin(node);
- ctx.setExceptionHandled(true);
- }
- }
- throw e;
- } catch (Exception e) {
- if (node != null) {
+ } catch (Throwable e) {
+ if (!ctx.isExceptionHandled() && node != null) {
if (isReadLatched) {
node.releaseReadLatch();
} else {
@@ -756,9 +741,8 @@
}
bufferCache.unpin(node);
}
- HyracksDataException wrappedException = HyracksDataException.create(e);
ctx.setExceptionHandled(true);
- throw wrappedException;
+ throw HyracksDataException.create(e);
}
}
@@ -785,7 +769,7 @@
ICachedPage node = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), pageId), false);
node.acquireReadLatch();
try {
- if (parent != null && unpin == true) {
+ if (parent != null && unpin) {
parent.releaseReadLatch();
bufferCache.unpin(parent);
}
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java
index 1491424..71df593 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java
@@ -119,13 +119,21 @@
private void fetchNextLeafPage(int nextLeafPage) throws HyracksDataException {
do {
- ICachedPage nextLeaf = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, nextLeafPage), false);
- if (exclusiveLatchNodes) {
- nextLeaf.acquireWriteLatch();
- page.releaseWriteLatch(isPageDirty);
- } else {
- nextLeaf.acquireReadLatch();
- page.releaseReadLatch();
+ final ICachedPage nextLeaf;
+ try {
+ nextLeaf = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, nextLeafPage), false);
+ if (exclusiveLatchNodes) {
+ nextLeaf.acquireWriteLatch();
+ } else {
+ nextLeaf.acquireReadLatch();
+ }
+ } finally {
+ // release latches in finally, don't leak locks on pin failure
+ if (exclusiveLatchNodes) {
+ page.releaseWriteLatch(isPageDirty);
+ } else {
+ page.releaseReadLatch();
+ }
}
bufferCache.unpin(page);
page = nextLeaf;
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java
index 9ccd881..cf3727a 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java
@@ -110,8 +110,13 @@
protected void fetchNextLeafPage(int nextLeafPage) throws HyracksDataException {
do {
- ICachedPage nextLeaf = acquirePage(nextLeafPage);
- releasePage();
+ ICachedPage nextLeaf;
+ try {
+ nextLeaf = acquirePage(nextLeafPage);
+ } finally {
+ // release page in finally, don't leak lock on pin failure
+ releasePage();
+ }
page = nextLeaf;
isPageDirty = false;
frame.setPage(page);
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java
index ae6bbaa..282aad9 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java
@@ -143,19 +143,12 @@
ctx.getCursorInitialState().setPageId(childPageId);
ctx.getLeafFrame().setPage(currentPage);
cursor.open(ctx.getCursorInitialState(), ctx.getPred());
- } catch (HyracksDataException e) {
+ } catch (Exception e) {
if (!ctx.isExceptionHandled() && currentPage != null) {
bufferCache.unpin(currentPage);
- ctx.setExceptionHandled(true);
}
- throw e;
- } catch (Exception e) {
- if (currentPage != null) {
- bufferCache.unpin(currentPage);
- }
- HyracksDataException wrappedException = HyracksDataException.create(e);
ctx.setExceptionHandled(true);
- throw wrappedException;
+ throw HyracksDataException.create(e);
}
}
diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualFreePageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualFreePageManager.java
index 9d62c0d..0e4f835 100644
--- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualFreePageManager.java
+++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualFreePageManager.java
@@ -104,10 +104,13 @@
page = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, currentPageId.get()), true);
if (leafFrameFactory != null) {
page.acquireWriteLatch();
- ITreeIndexFrame leafFrame = leafFrameFactory.createFrame();
- leafFrame.setPage(page);
- leafFrame.initBuffer((byte) 0);
- page.releaseWriteLatch(true);
+ try {
+ ITreeIndexFrame leafFrame = leafFrameFactory.createFrame();
+ leafFrame.setPage(page);
+ leafFrame.initBuffer((byte) 0);
+ } finally {
+ page.releaseWriteLatch(true);
+ }
}
bufferCache.unpin(page);
}