You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@asterixdb.apache.org by AsterixDB Code Review <do...@asterix-gerrit.ics.uci.edu> on 2024/02/27 00:19:58 UTC
Change in asterixdb[neo]: [NO ISSUE][HYR][STO] BufferCache lock fixes
From Michael Blow <mb...@apache.org>:
Michael Blow has uploaded this change for review. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18181 )
Change subject: [NO ISSUE][HYR][STO] BufferCache lock fixes
......................................................................
[NO ISSUE][HYR][STO] BufferCache lock fixes
Change-Id: I37f06163bbf1c34392d83a8ccd27e777552eeac7
---
M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/freepage/VirtualFreePageManager.java
M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java
M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java
M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java
M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java
5 files changed, 70 insertions(+), 67 deletions(-)
git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb refs/changes/81/18181/1
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..d33fd38 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());
@@ -221,8 +220,8 @@
ICachedPage originalPage = ctx.getInteriorFrame().getPage();
for (int i = 0; i < ctx.getSmPages().size(); i++) {
int pageId = ctx.getSmPages().get(i);
- ICachedPage smPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), pageId), false);
- smPage.acquireWriteLatch();
+ final ICachedPage smPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), pageId), false);
+ smPage.acquireWriteLatch(); // MDB: safe
try {
ctx.getInteriorFrame().setPage(smPage);
ctx.getInteriorFrame().setSmFlag(false);
@@ -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();
}
@@ -245,13 +240,14 @@
private void createNewRoot(BTreeOpContext ctx) throws HyracksDataException {
// Make sure the root is always in the same page.
- ICachedPage leftNode =
+ final ICachedPage leftNode =
bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), ctx.getSplitKey().getLeftPage()), false);
- leftNode.acquireWriteLatch();
+ leftNode.acquireWriteLatch(); // MDB: safe
try {
int newLeftId = freePageManager.takePage(ctx.getMetaFrame());
- ICachedPage newLeftNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), newLeftId), true);
- newLeftNode.acquireWriteLatch();
+ final ICachedPage newLeftNode =
+ bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), newLeftId), true);
+ newLeftNode.acquireWriteLatch(); // MDB: safe
try {
boolean largePage = false;
if (leftNode.getBuffer().capacity() > newLeftNode.getBuffer().capacity()) {
@@ -358,8 +354,8 @@
}
}
int rightPageId = freePageManager.takePage(ctx.getMetaFrame());
- ICachedPage rightNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), rightPageId), true);
- rightNode.acquireWriteLatch();
+ final ICachedPage rightNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), rightPageId), true);
+ rightNode.acquireWriteLatch(); // MDB: safe
try {
IBTreeLeafFrame rightFrame = ctx.createLeafFrame();
rightFrame.setPage(rightNode);
@@ -477,9 +473,9 @@
switch (spaceStatus) {
case INSUFFICIENT_SPACE: {
int rightPageId = freePageManager.takePage(ctx.getMetaFrame());
- ICachedPage rightNode =
+ final ICachedPage rightNode =
bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), rightPageId), true);
- rightNode.acquireWriteLatch();
+ rightNode.acquireWriteLatch(); // MDB: safe
try {
IBTreeFrame rightFrame = ctx.createInteriorFrame();
rightFrame.setPage(rightNode);
@@ -599,8 +595,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 +610,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).
@@ -632,9 +631,9 @@
case UPDATE: {
// Is there a propagated split key?
if (ctx.getSplitKey().getBuffer() != null) {
- ICachedPage interiorNode = bufferCache
+ final ICachedPage interiorNode = bufferCache
.pin(BufferedFileHandle.getDiskPageId(getFileId(), pageId), false);
- interiorNode.acquireWriteLatch();
+ interiorNode.acquireWriteLatch(); // MDB: safe
try {
// Insert or update op. Both can cause split keys to propagate upwards.
insertInterior(interiorNode, pageId, ctx.getSplitKey().getTuple(), ctx);
@@ -662,7 +661,7 @@
}
}
// Operation completed.
- repeatOp = false;
+ break;
} // end while
} else { // smFlag
ctx.setOpRestarts(ctx.getOpRestarts() + 1);
@@ -734,21 +733,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 {
@@ -782,10 +768,10 @@
public void printTree(int pageId, ICachedPage parent, boolean unpin, IBTreeLeafFrame leafFrame,
IBTreeInteriorFrame interiorFrame, byte treeHeight, ISerializerDeserializer[] keySerdes,
StringBuilder strBuilder, MultiComparator cmp) throws Exception {
- ICachedPage node = bufferCache.pin(BufferedFileHandle.getDiskPageId(getFileId(), pageId), false);
+ final 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..7637bdf 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,20 @@
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 {
+ 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..7282e9d 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,12 @@
protected void fetchNextLeafPage(int nextLeafPage) throws HyracksDataException {
do {
- ICachedPage nextLeaf = acquirePage(nextLeafPage);
- releasePage();
+ ICachedPage nextLeaf;
+ try {
+ nextLeaf = acquirePage(nextLeafPage);
+ } finally {
+ 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..e9aeae4 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,14 +143,8 @@
ctx.getCursorInitialState().setPageId(childPageId);
ctx.getLeafFrame().setPage(currentPage);
cursor.open(ctx.getCursorInitialState(), ctx.getPred());
- } catch (HyracksDataException e) {
- if (!ctx.isExceptionHandled() && currentPage != null) {
- bufferCache.unpin(currentPage);
- ctx.setExceptionHandled(true);
- }
- throw e;
} catch (Exception e) {
- if (currentPage != null) {
+ if (!ctx.isExceptionHandled() && currentPage != null) {
bufferCache.unpin(currentPage);
}
HyracksDataException wrappedException = 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..36e967b 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
@@ -98,16 +98,19 @@
throws HyracksDataException {
currentPageId.set(1);
ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, 0), true);
- page.acquireWriteLatch();
+ page.acquireWriteLatch(); // MDB: safe
page.releaseWriteLatch(false);
bufferCache.unpin(page);
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);
+ page.acquireWriteLatch(); // MDB: safe
+ try {
+ ITreeIndexFrame leafFrame = leafFrameFactory.createFrame();
+ leafFrame.setPage(page);
+ leafFrame.initBuffer((byte) 0);
+ } finally {
+ page.releaseWriteLatch(true);
+ }
}
bufferCache.unpin(page);
}
@@ -130,8 +133,8 @@
@Override
public boolean isEmpty(ITreeIndexFrame frame, int rootPage) throws HyracksDataException {
- ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false);
- rootNode.acquireReadLatch();
+ final ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false);
+ rootNode.acquireReadLatch(); // MDB: safe
try {
frame.setPage(rootNode);
return frame.getLevel() == 0 && frame.getTupleCount() == 0;
--
To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/18181
To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings
Gerrit-Project: asterixdb
Gerrit-Branch: neo
Gerrit-Change-Id: I37f06163bbf1c34392d83a8ccd27e777552eeac7
Gerrit-Change-Number: 18181
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Blow <mb...@apache.org>
Gerrit-MessageType: newchange