>From Michael Blow <[email protected]>: Michael Blow has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19883 )
Change subject: [NO ISSUE][STO][MISC] Fixes to handling of interrupted actions ...................................................................... [NO ISSUE][STO][MISC] Fixes to handling of interrupted actions - Avoid double unpin on interrupt during unwind of pipeline by clearing the pinned page after successful unpin. - Propagate the interrupted exception when feasible when InvokeUtil helpers are used- previously these helpers would only set the interrupted flag on the current thread which can mask interrupted actions. (cherry picked from commit 65b6a27d1) Ext-ref: MB-67050 Change-Id: I2f8c64fa76beecbebcb40cf2a86866189cb1c55a Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19883 Tested-by: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> Reviewed-by: Michael Blow <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageFileAccessTest.java M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/TreeIndexDiskOrderScanCursor.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java M asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeRangeSearchCursor.java 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-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/fixedsize/FixedSizeElementInvertedListScanCursor.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/DiskBTreePointSearchCursor.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/AbstractOnDiskInvertedListCursor.java M hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java M hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java M hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/InvokeUtil.java M hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTree.java 17 files changed, 209 insertions(+), 56 deletions(-) Approvals: Michael Blow: Looks good to me, but someone else must approve Ali Alsuliman: Looks good to me, approved Jenkins: Verified diff --git a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java index 1565730..f877143 100644 --- a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java +++ b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/GlobalVirtualBufferCache.java @@ -313,7 +313,7 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { vbc.unpin(page); } diff --git a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/InvokeUtil.java b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/InvokeUtil.java index d331ab2..a680ee1 100644 --- a/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/InvokeUtil.java +++ b/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/InvokeUtil.java @@ -35,6 +35,7 @@ import org.apache.hyracks.util.InterruptibleSupplier; import org.apache.hyracks.util.Span; import org.apache.hyracks.util.ThrowingAction; +import org.apache.hyracks.util.ThrowingConsumer; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -53,7 +54,12 @@ /** * Executes the passed interruptible, retrying if the operation is interrupted. Once the interruptible * completes, the current thread will be re-interrupted, if the original operation was interrupted. + * + * @deprecated this method does not throw an exception when the action is interrupted, or when the thread + * was interrupted prior to calling this method. This can lead to confusing behavior, if the caller + * does not check for interrupted thread state after calling this method. */ + @Deprecated public static void doUninterruptibly(InterruptibleAction interruptible) { boolean interrupted = Thread.interrupted(); try { @@ -211,10 +217,11 @@ } } - @SuppressWarnings({ "squid:S1181", "squid:S1193", "ConstantConditions" }) // catching Throwable, instanceofs - public static void tryWithCleanups(ThrowingAction action, ThrowingAction... cleanups) throws Exception { + @SuppressWarnings({ "squid:S1181", "squid:S1193", "ConstantConditions", "UnreachableCode" }) + // catching Throwable, instanceofs, false-positive unreachable code + public static void tryWithCleanupsAsHyracks(ThrowingAction action, ThrowingAction... cleanups) + throws HyracksDataException { Throwable savedT = null; - boolean suppressedInterrupted = false; try { action.run(); } catch (Throwable t) { @@ -224,6 +231,137 @@ try { cleanup.run(); } catch (Throwable t) { + savedT = ExceptionUtils.suppress(savedT, t); + } + } + } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } + if (savedT == null) { + return; + } + throw HyracksDataException.create(savedT); + } + + @SuppressWarnings({ "squid:S1181", "squid:S1193", "ConstantConditions", "UnreachableCode" }) + // catching Throwable, instanceofs, false-positive unreachable code + public static void tryWithCleanups(ThrowingAction action, ThrowingAction... cleanups) throws Exception { + Throwable savedT = null; + try { + action.run(); + } catch (Throwable t) { + savedT = t; + } finally { + for (ThrowingAction cleanup : cleanups) { + try { + cleanup.run(); + } catch (Throwable t) { + savedT = ExceptionUtils.suppress(savedT, t); + } + } + } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } + if (savedT == null) { + return; + } + if (savedT instanceof Error) { + throw (Error) savedT; + } else if (savedT instanceof Exception) { + throw (Exception) savedT; + } else { + throw HyracksDataException.create(savedT); + } + } + + /** + * Runs the supplied action, and any specified cleanups. Any pending interruption will be cleared prior + * to running the action and all cleanups. An error will be logged if the action and/or any of the cleanups + * are themselves interrupted. Finally, if any action or cleanup was interrupted, or if the there was an + * interrupt cleared as part of running any of these activities, either an InterruptedException will be returned + * if the action & cleanups all ran without exception, or an InterruptedException will be suppressed into the + * exception if the action or any of the cleanups threw an exception. In the case where InterruptedException is + * suppressed, the current thread will be interrupted. + * + * @param action the action to run + * @param cleanups the cleanups to run after the action + * @return Exception if the action throws an exception or the action or any of the cleanups are interrupted, or if + * the current thread was interrupted before running the action or any of the cleanups. + */ + @SuppressWarnings({ "squid:S1181", "squid:S1193", "ConstantConditions", "UnreachableCode" }) + // catching Throwable, instanceofs, false-positive unreachable code + public static Exception tryUninterruptibleWithCleanups(Exception root, ThrowingAction action, + ThrowingAction... cleanups) { + try { + tryUninterruptibleWithCleanups(action, cleanups); + } catch (Exception e) { + root = ExceptionUtils.suppress(root, e); + } + return root; + } + + /** + * Runs the supplied action, and any specified cleanups. Any pending interruption will be cleared prior + * to running the action and all cleanups. An error will be logged if the action and/or any of the cleanups + * are themselves interrupted. Finally, if any action or cleanup was interrupted, or if the there was an + * interrupt cleared as part of running any of these activities, either an InterruptedException will be thrown + * if the action & cleanups all ran without exception, or an InterruptedException will be suppressed into the + * exception if the action or any of the cleanups threw an exception. In the case where InterruptedException is + * suppressed, the current thread will be interrupted. + * + * @param action the action to run + * @param cleanups the cleanups to run after the action + * @throws Exception if the action throws an exception or the action or any of the cleanups are interrupted. + */ + @SuppressWarnings({ "squid:S1181", "squid:S1193", "ConstantConditions", "UnreachableCode" }) + // catching Throwable, instanceofs, false-positive unreachable code + public static void tryUninterruptibleWithCleanups(ThrowingAction action, ThrowingAction... cleanups) + throws Exception { + Throwable savedT = null; + try { + runUninterruptible(action); + } catch (Throwable t) { + savedT = t; + } finally { + for (ThrowingAction cleanup : cleanups) { + try { + runUninterruptible(cleanup); + } catch (Throwable t) { + savedT = ExceptionUtils.suppress(savedT, t); + } + } + } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } + if (savedT == null) { + return; + } + if (savedT instanceof Error) { + throw (Error) savedT; + } else if (savedT instanceof Exception) { + throw (Exception) savedT; + } else { + throw HyracksDataException.create(savedT); + } + } + + // catching Throwable, instanceofs, false-positive unreachable code + public static void tryWithCleanups(ThrowingAction action, ThrowingConsumer<Throwable>... cleanups) + throws Exception { + Throwable savedT = null; + boolean suppressedInterrupted = false; + try { + action.run(); + } catch (Throwable t) { + savedT = t; + } finally { + for (ThrowingConsumer cleanup : cleanups) { + try { + cleanup.process(savedT); + } catch (Throwable t) { if (savedT != null) { savedT.addSuppressed(t); suppressedInterrupted = suppressedInterrupted || t instanceof InterruptedException; @@ -329,16 +467,12 @@ boolean interrupted = Thread.interrupted(); try { action.run(); - if (Thread.interrupted()) { + if (interrupted || Thread.interrupted()) { throw new InterruptedException(); } } catch (InterruptedException e) { LOGGER.error("uninterruptible action {} was interrupted!", action, e); - interrupted = true; - } finally { - if (interrupted) { - Thread.currentThread().interrupt(); - } + throw e; } } 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 71df593..39e2a38 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 @@ -108,13 +108,14 @@ stopTupleIndex = getHighKeyIndex(); } - private void releasePage() throws HyracksDataException { + private void releasePage() { if (exclusiveLatchNodes) { page.releaseWriteLatch(isPageDirty); } else { page.releaseReadLatch(); } bufferCache.unpin(page); + page = null; } private void fetchNextLeafPage(int nextLeafPage) throws HyracksDataException { @@ -218,7 +219,6 @@ } tupleBuilder.reset(); tupleIndex = 0; - page = null; isPageDirty = false; pred = null; count = -1; 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 cf3727a..99e2d0b 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 @@ -46,7 +46,6 @@ protected final IBTreeLeafFrame frame; protected final ITreeIndexTupleReference frameTuple; protected final boolean exclusiveLatchNodes; - protected boolean isPageDirty; protected IBufferCache bufferCache = null; protected int fileId = -1; @@ -118,7 +117,6 @@ releasePage(); } page = nextLeaf; - isPageDirty = false; frame.setPage(page); pageId = nextLeafPage; nextLeafPage = frame.getNextLeaf(); @@ -159,8 +157,6 @@ reconciliationTuple.reset(tupleBuilder.getFieldEndOffsets(), tupleBuilder.getByteArray()); releasePage(); - page = null; - isPageDirty = false; // reconcile searchCb.reconcile(reconciliationTuple); @@ -236,7 +232,6 @@ originalKeyCmp = initialState.getOriginalKeyComparator(); pageId = ((BTreeCursorInitialState) initialState).getPageId(); page = initialState.getPage(); - isPageDirty = false; frame.setPage(page); pred = (RangePredicate) searchPred; @@ -267,7 +262,7 @@ stopTupleIndex = getHighKeyIndex(); } - protected void resetBeforeOpen() throws HyracksDataException { + protected void resetBeforeOpen() { releasePage(); } @@ -278,8 +273,6 @@ } tupleIndex = 0; - page = null; - isPageDirty = false; pred = null; } @@ -298,13 +291,17 @@ return exclusiveLatchNodes; } - protected void releasePage() throws HyracksDataException { + /** + * Releases the page and unpins it from the buffer cache, clearing the reference to the page. + */ + protected void releasePage() { if (exclusiveLatchNodes) { - page.releaseWriteLatch(isPageDirty); + page.releaseWriteLatch(false); } else { page.releaseReadLatch(); } bufferCache.unpin(page); + page = null; } protected ICachedPage acquirePage(int pageId) throws HyracksDataException { 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 282aad9..1cbe31d 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 @@ -222,7 +222,7 @@ } @Override - protected void releasePage() throws HyracksDataException { + protected void releasePage() { if (page != null) { bufferCache.unpin(page); page = null; diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreePointSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreePointSearchCursor.java index 1bf3ecf..a1f04c6 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreePointSearchCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreePointSearchCursor.java @@ -69,7 +69,6 @@ originalKeyCmp = initialState.getOriginalKeyComparator(); pageId = ((BTreeCursorInitialState) initialState).getPageId(); page = initialState.getPage(); - isPageDirty = false; frame.setPage(page); pred = (RangePredicate) searchPred; diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeRangeSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeRangeSearchCursor.java index d788398..d62ca91 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeRangeSearchCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/DiskBTreeRangeSearchCursor.java @@ -66,8 +66,9 @@ } @Override - protected void releasePage() throws HyracksDataException { + protected void releasePage() { bufferCache.unpin(page); + page = null; } @Override diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/TreeIndexDiskOrderScanCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/TreeIndexDiskOrderScanCursor.java index 36fba76..b0a2c71 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/TreeIndexDiskOrderScanCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/TreeIndexDiskOrderScanCursor.java @@ -142,7 +142,10 @@ return false; } - protected void releasePage() throws HyracksDataException { + /** + * Releases the current page, if it is not null, clearing the reference to it. + */ + protected void releasePage() { if (page != null) { page.releaseReadLatch(); bufferCache.unpin(page); diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java index e77acea..f801025 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MultitenantVirtualBufferCache.java @@ -70,7 +70,7 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { vbc.unpin(page); } 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 5871b31..60b38dd 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 @@ -317,7 +317,7 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { } @Override diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/AbstractOnDiskInvertedListCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/AbstractOnDiskInvertedListCursor.java index 9886da4..b68a163 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/AbstractOnDiskInvertedListCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/AbstractOnDiskInvertedListCursor.java @@ -219,6 +219,7 @@ currentBufferIdx++; bufferCache.unpin(page); + page = null; bufferEndPageId = i; // Buffer full? diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/fixedsize/FixedSizeElementInvertedListScanCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/fixedsize/FixedSizeElementInvertedListScanCursor.java index 458eb6b..cdc0701 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/fixedsize/FixedSizeElementInvertedListScanCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/fixedsize/FixedSizeElementInvertedListScanCursor.java @@ -127,7 +127,7 @@ } @Override - public void unloadPages() throws HyracksDataException { + public void unloadPages() { if (pinned) { bufferCache.unpin(page); pinned = false; diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java index d7be126..cf3c104 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java @@ -573,9 +573,9 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { if (closed) { - throw new HyracksDataException("unpin called on a closed cache"); + throw new IllegalStateException("unpin called on a closed cache"); } int pinCount = ((CachedPage) page).decrementAndGetPinCount(); if (DEBUG && pinCount == 0) { diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java index 8c3d492..88f3035 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/DebugBufferCache.java @@ -84,7 +84,7 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { bufferCache.unpin(page); unpinCount.addAndGet(1); } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java index df0bea8..f133d80 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IBufferCache.java @@ -109,9 +109,8 @@ * * @param page * the page - * @throws HyracksDataException */ - void unpin(ICachedPage page) throws HyracksDataException; + void unpin(ICachedPage page); /** * Flush the page if it is dirty diff --git a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageFileAccessTest.java b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageFileAccessTest.java index 576c6bb..226d3ed 100644 --- a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageFileAccessTest.java +++ b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageFileAccessTest.java @@ -143,31 +143,27 @@ private void unpinRandomPage() { int index = Math.abs(rnd.nextInt() % pinnedPages.size()); - try { - PinnedLatchedPage plPage = pinnedPages.get(index); + PinnedLatchedPage plPage = pinnedPages.get(index); - if (plPage.latch != null) { - if (plPage.latch == LatchType.LATCH_S) { - if (LOGGER.isInfoEnabled()) { - LOGGER.info(workerId + " S UNLATCHING: " + plPage.pageId); - } - plPage.page.releaseReadLatch(); - } else { - if (LOGGER.isInfoEnabled()) { - LOGGER.info(workerId + " X UNLATCHING: " + plPage.pageId); - } - plPage.page.releaseWriteLatch(true); + if (plPage.latch != null) { + if (plPage.latch == LatchType.LATCH_S) { + if (LOGGER.isInfoEnabled()) { + LOGGER.info(workerId + " S UNLATCHING: " + plPage.pageId); } + plPage.page.releaseReadLatch(); + } else { + if (LOGGER.isInfoEnabled()) { + LOGGER.info(workerId + " X UNLATCHING: " + plPage.pageId); + } + plPage.page.releaseWriteLatch(true); } - if (LOGGER.isInfoEnabled()) { - LOGGER.info(workerId + " UNPINNING PAGE: " + plPage.pageId); - } - - bufferCache.unpin(plPage.page); - pinnedPages.remove(index); - } catch (HyracksDataException e) { - e.printStackTrace(); } + if (LOGGER.isInfoEnabled()) { + LOGGER.info(workerId + " UNPINNING PAGE: " + plPage.pageId); + } + + bufferCache.unpin(plPage.page); + pinnedPages.remove(index); } private void openFile() { diff --git a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java index 8cad497..d7a83dc 100644 --- a/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java +++ b/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-lsm-btree-test/src/test/java/org/apache/hyracks/storage/am/lsm/btree/impl/TestVirtualBufferCache.java @@ -101,7 +101,7 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { vbc.unpin(page); } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19883 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: I2f8c64fa76beecbebcb40cf2a86866189cb1c55a Gerrit-Change-Number: 19883 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Blow <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-MessageType: merged
