>From Michael Blow <[email protected]>: Michael Blow has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19843 )
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 65b6a27d15) Ext-ref: MB-66894 Change-Id: I2f8c64fa76beecbebcb40cf2a86866189cb1c55a Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19843 Reviewed-by: Michael Blow <[email protected]> Reviewed-by: Hussain Towaileb <[email protected]> Tested-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-cloud/src/main/java/org/apache/hyracks/cloud/buffercache/context/DefaultCloudReadContext.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-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/lsm/tuples/ColumnMultiBufferProvider.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 19 files changed, 123 insertions(+), 92 deletions(-) Approvals: Michael Blow: Looks good to me, but someone else must approve; Verified Hussain Towaileb: Looks good to me, approved 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 3eb1e4c..cb5323f 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 @@ -326,12 +326,12 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { unpin(page, DEFAULT); } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { vbc.unpin(page, context); } 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 9326427..2a77bab 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 @@ -56,7 +56,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 { @@ -219,7 +224,6 @@ public static void tryWithCleanupsAsHyracks(ThrowingAction action, ThrowingAction... cleanups) throws HyracksDataException { Throwable savedT = null; - boolean suppressedInterrupted = false; try { action.run(); } catch (Throwable t) { @@ -229,21 +233,16 @@ try { cleanup.run(); } catch (Throwable t) { - if (savedT != null) { - savedT.addSuppressed(t); - suppressedInterrupted |= t instanceof InterruptedException; - } else { - savedT = t; - } + savedT = ExceptionUtils.suppress(savedT, t); } } } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } if (savedT == null) { return; } - if (suppressedInterrupted) { - Thread.currentThread().interrupt(); - } throw HyracksDataException.create(savedT); } @@ -251,7 +250,6 @@ // catching Throwable, instanceofs, false-positive unreachable code public static void tryWithCleanups(ThrowingAction action, ThrowingAction... cleanups) throws Exception { Throwable savedT = null; - boolean suppressedInterrupted = false; try { action.run(); } catch (Throwable t) { @@ -261,21 +259,16 @@ try { cleanup.run(); } catch (Throwable t) { - if (savedT != null) { - savedT.addSuppressed(t); - suppressedInterrupted = suppressedInterrupted || t instanceof InterruptedException; - } else { - savedT = t; - } + savedT = ExceptionUtils.suppress(savedT, t); } } } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } if (savedT == null) { return; } - if (suppressedInterrupted) { - Thread.currentThread().interrupt(); - } if (savedT instanceof Error) { throw (Error) savedT; } else if (savedT instanceof Exception) { @@ -285,6 +278,20 @@ } } + /** + * 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, @@ -297,12 +304,24 @@ 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; - boolean suppressedInterrupted = false; try { runUninterruptible(action); } catch (Throwable t) { @@ -312,21 +331,16 @@ try { runUninterruptible(cleanup); } catch (Throwable t) { - if (savedT != null) { - savedT.addSuppressed(t); - suppressedInterrupted = suppressedInterrupted || t instanceof InterruptedException; - } else { - savedT = t; - } + savedT = ExceptionUtils.suppress(savedT, t); } } } + if (Thread.interrupted()) { + savedT = ExceptionUtils.suppress(savedT, new InterruptedException()); + } if (savedT == null) { return; } - if (suppressedInterrupted) { - Thread.currentThread().interrupt(); - } if (savedT instanceof Error) { throw (Error) savedT; } else if (savedT instanceof Exception) { @@ -494,16 +508,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-cloud/src/main/java/org/apache/hyracks/cloud/buffercache/context/DefaultCloudReadContext.java b/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/buffercache/context/DefaultCloudReadContext.java index dab23d9..fc66958 100644 --- a/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/buffercache/context/DefaultCloudReadContext.java +++ b/hyracks-fullstack/hyracks/hyracks-cloud/src/main/java/org/apache/hyracks/cloud/buffercache/context/DefaultCloudReadContext.java @@ -30,6 +30,7 @@ import org.apache.hyracks.storage.common.buffercache.BufferCache; import org.apache.hyracks.storage.common.buffercache.BufferCacheHeaderHelper; import org.apache.hyracks.storage.common.buffercache.CachedPage; +import org.apache.hyracks.storage.common.buffercache.IBufferCache; import org.apache.hyracks.storage.common.buffercache.ICachedPage; import org.apache.hyracks.storage.common.buffercache.context.IBufferCacheReadContext; import org.apache.hyracks.storage.common.disk.IPhysicalDrive; @@ -39,7 +40,7 @@ /** * Default context for {@link BufferCache#pin(long, IBufferCacheReadContext)} - * and {@link BufferCache#unpin(ICachedPage, IBufferCacheReadContext)} in a cloud deployment. + * and {@link IBufferCache#unpin(ICachedPage, IBufferCacheReadContext)} in a cloud deployment. * <p> * The default behavior of this context is persisting the pages read from cloud if {@link IPhysicalDrive} * reports that the local drive(s) are not pressured. 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 a010fde..e956622 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 034a714..50ecb53 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 87b0032..fcfaa40 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 @@ -220,7 +220,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 8fd9a96..beadcf1 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 @@ -76,7 +76,6 @@ originalKeyCmp = initialState.getOriginalKeyComparator(); pageId = ((BTreeCursorInitialState) initialState).getPageId(); page = initialState.getPage(); - isPageDirty = false; frame.setPage(page); setCursorToNextKey(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 3dc4df7..683bbf8 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 14decb4..f637ac7 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-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/lsm/tuples/ColumnMultiBufferProvider.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/lsm/tuples/ColumnMultiBufferProvider.java index 2a2926f..22ef4f1 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/lsm/tuples/ColumnMultiBufferProvider.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree-column/src/main/java/org/apache/hyracks/storage/am/lsm/btree/column/impls/lsm/tuples/ColumnMultiBufferProvider.java @@ -94,8 +94,8 @@ @Override public void releaseAll() throws HyracksDataException { - while (!pages.isEmpty()) { - ICachedPage page = pages.poll(); + ICachedPage page; + while ((page = pages.poll()) != null) { multiPageOp.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 f42d0d3..7f4fd38 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 @@ -79,12 +79,12 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { unpin(page, DEFAULT); } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { vbc.unpin(page, context); } 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 564a9b7..66c596d 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 @@ -327,11 +327,11 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { } @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 c5ad6eb..460ac20 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 @@ -220,6 +220,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 f237865..f570bd8 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 a4ecb6f..a77ccdf 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 @@ -601,14 +601,14 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { unpin(page, defaultContext); } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { if (closed) { - throw new HyracksDataException("unpin called on a closed cache"); + throw new IllegalStateException("unpin called on a closed cache"); } context.onUnpin(page); 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 342d003..f1298b7 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 @@ -93,12 +93,12 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { unpin(page, DEFAULT); } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { bufferCache.unpin(page, context); 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 d69ded1..81d6d56 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 @@ -105,7 +105,7 @@ * * @param page the page */ - void unpin(ICachedPage page) throws HyracksDataException; + void unpin(ICachedPage page); /** * Unpin a page to indicate can be evicted @@ -114,7 +114,7 @@ * @param page the page * @param context read context */ - void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException; + void unpin(ICachedPage page, IBufferCacheReadContext context); /** * 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 bdcb206..756a29f 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 637e553..d4ea5a0 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 @@ -111,12 +111,12 @@ } @Override - public void unpin(ICachedPage page) throws HyracksDataException { + public void unpin(ICachedPage page) { unpin(page, DEFAULT); } @Override - public void unpin(ICachedPage page, IBufferCacheReadContext context) throws HyracksDataException { + public void unpin(ICachedPage page, IBufferCacheReadContext context) { vbc.unpin(page, context); } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/19843 To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-Project: asterixdb Gerrit-Branch: stabilization-27a661be67 Gerrit-Change-Id: I2f8c64fa76beecbebcb40cf2a86866189cb1c55a Gerrit-Change-Number: 19843 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Blow <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-MessageType: merged
