Repository: asterixdb Updated Branches: refs/heads/master fa6c95941 -> dba817c9b
[NO ISSUE][RT] Ensure an inverted list cursor.close() - user model changes: no - storage format changes: no - interface changes: no details: - Ensure to always execute an inverted list cursor.close(). Change-Id: I78c7908830be810b1d40abffffbd5f1978818869 Reviewed-on: https://asterix-gerrit.ics.uci.edu/2410 Sonar-Qube: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Integration-Tests: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Reviewed-by: abdullah alamoudi <bamou...@gmail.com> Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/dba817c9 Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/dba817c9 Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/dba817c9 Branch: refs/heads/master Commit: dba817c9be00226fd0810d6dcb6673e46ed0384a Parents: fa6c959 Author: Taewoo Kim <wangs...@yahoo.com> Authored: Wed Feb 21 11:18:14 2018 -0800 Committer: Taewoo Kim <wangs...@gmail.com> Committed: Wed Feb 21 14:33:29 2018 -0800 ---------------------------------------------------------------------- .../inmemory/InMemoryInvertedListCursor.java | 1 + .../OnDiskInvertedIndexRangeSearchCursor.java | 39 +++++++--- .../search/AbstractTOccurrenceSearcher.java | 16 ++-- .../search/InvertedListMerger.java | 82 ++++++++++++-------- 4 files changed, 88 insertions(+), 50 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/asterixdb/blob/dba817c9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/inmemory/InMemoryInvertedListCursor.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/inmemory/InMemoryInvertedListCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/inmemory/InMemoryInvertedListCursor.java index 085f8d5..6e8689b 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/inmemory/InMemoryInvertedListCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/inmemory/InMemoryInvertedListCursor.java @@ -172,6 +172,7 @@ public class InMemoryInvertedListCursor extends InvertedListCursor { btreePred.setHighKey(btreeSearchTuple, true); try { btreeAccessor.search(btreeCursor, btreePred); + cursorNeedsClose = true; } catch (Exception e) { btreeSearchTuple.removeLastTuple(); throw HyracksDataException.create(e); http://git-wip-us.apache.org/repos/asf/asterixdb/blob/dba817c9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java index a33b045..d9e7d34 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndexRangeSearchCursor.java @@ -87,8 +87,11 @@ public class OnDiskInvertedIndexRangeSearchCursor extends EnforcedIndexCursor { return true; } // The current inverted-list-range-search cursor is exhausted. - invListRangeSearchCursor.unloadPages(); - invListRangeSearchCursor.close(); + try { + invListRangeSearchCursor.unloadPages(); + } finally { + invListRangeSearchCursor.close(); + } isInvListCursorOpen = false; openInvListRangeSearchCursor(); return isInvListCursorOpen; @@ -105,22 +108,34 @@ public class OnDiskInvertedIndexRangeSearchCursor extends EnforcedIndexCursor { @Override public void doDestroy() throws HyracksDataException { - if (isInvListCursorOpen) { - invListRangeSearchCursor.unloadPages(); - invListRangeSearchCursor.destroy(); - isInvListCursorOpen = false; + try { + if (isInvListCursorOpen) { + try { + invListRangeSearchCursor.unloadPages(); + } finally { + isInvListCursorOpen = false; + invListRangeSearchCursor.destroy(); + } + } + } finally { + btreeCursor.destroy(); } - btreeCursor.destroy(); } @Override public void doClose() throws HyracksDataException { - if (isInvListCursorOpen) { - invListRangeSearchCursor.unloadPages(); - invListRangeSearchCursor.close(); - isInvListCursorOpen = false; + try { + if (isInvListCursorOpen) { + try { + invListRangeSearchCursor.unloadPages(); + } finally { + invListRangeSearchCursor.close(); + } + isInvListCursorOpen = false; + } + } finally { + btreeCursor.close(); } - btreeCursor.close(); } @Override http://git-wip-us.apache.org/repos/asf/asterixdb/blob/dba817c9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java index ff9a2f1..cd0ba36 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/AbstractTOccurrenceSearcher.java @@ -239,8 +239,11 @@ public abstract class AbstractTOccurrenceSearcher implements IInvertedIndexSearc private void resetResultSource() throws HyracksDataException { if (isSingleInvertedList) { isSingleInvertedList = false; - singleInvListCursor.unloadPages(); - singleInvListCursor.close(); + try { + singleInvListCursor.unloadPages(); + } finally { + singleInvListCursor.close(); + } singleInvListCursor = null; } else { finalSearchResult.resetBuffer(); @@ -253,9 +256,12 @@ public abstract class AbstractTOccurrenceSearcher implements IInvertedIndexSearc ((BufferManagerBackedVSizeFrame) queryTokenFrame).destroy(); // Releases the frames of the cursor. - if (isSingleInvertedList && singleInvListCursor != null) { - singleInvListCursor.unloadPages(); - singleInvListCursor.close(); + if (singleInvListCursor != null) { + try { + singleInvListCursor.unloadPages(); + } finally { + singleInvListCursor.close(); + } } // Releases the frame of the final search result. finalSearchResult.close(); http://git-wip-us.apache.org/repos/asf/asterixdb/blob/dba817c9/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedListMerger.java ---------------------------------------------------------------------- diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedListMerger.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedListMerger.java index 0bfc140..5da4702 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedListMerger.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedListMerger.java @@ -126,41 +126,48 @@ public class InvertedListMerger { isFinalList = true; } InvertedListCursor invListCursor = invListCursors.get(i); - // Loads the inverted list (at least some part of it). - invListCursor.prepareLoadPages(); - invListCursor.loadPages(); - if (i < numPrefixLists) { - // Merges a prefix list. - doneMerge = mergePrefixList(invListCursor, prevSearchResult, result, isFinalList); - } else { - // Merge suffix list. - int numInvListElements = invListCursor.size(); - int currentNumResults = prevSearchResult.getNumResults(); - // Should we binary search the next list or should we sort-merge it? - if (currentNumResults * Math.log(numInvListElements) < currentNumResults + numInvListElements) { - doneMerge = mergeSuffixListProbe(invListCursor, prevSearchResult, result, i, numInvLists, - occurrenceThreshold, isFinalList); + // Track whether an exception is occurred. + boolean finishedTryBlock = false; + try { + // Loads the inverted list (at least some part of it). + invListCursor.prepareLoadPages(); + invListCursor.loadPages(); + if (i < numPrefixLists) { + // Merges a prefix list. + doneMerge = mergePrefixList(invListCursor, prevSearchResult, result, isFinalList); } else { - doneMerge = mergeSuffixListScan(invListCursor, prevSearchResult, result, i, numInvLists, - occurrenceThreshold, isFinalList); + // Merge suffix list. + int numInvListElements = invListCursor.size(); + int currentNumResults = prevSearchResult.getNumResults(); + // Should we binary search the next list or should we sort-merge it? + if (currentNumResults * Math.log(numInvListElements) < currentNumResults + numInvListElements) { + doneMerge = mergeSuffixListProbe(invListCursor, prevSearchResult, result, i, numInvLists, + occurrenceThreshold, isFinalList); + } else { + doneMerge = mergeSuffixListScan(invListCursor, prevSearchResult, result, i, numInvLists, + occurrenceThreshold, isFinalList); + } + } + finishedTryBlock = true; + } finally { + // An intermediate inverted list should always be closed. + // The final inverted list should be closed only when traversing the list is done. + // If an exception was occurred, just close the cursor. + if (!isFinalList || (isFinalList && doneMerge) || !finishedTryBlock) { + try { + invListCursor.unloadPages(); + } finally { + invListCursor.close(); + } } } - if (isFinalList) { - // For the final list, the method unloadPages() should be called only when traversing - // the inverted list is finished. - if (doneMerge) { - invListCursor.unloadPages(); - invListCursor.close(); - } - // Needs to return the calculation result for the final list only. - // Otherwise, the process needs to be continued until this method traverses the final inverted list - // and either generates some output in the output buffer or finishes traversing it. + // Needs to return the calculation result for the final list only. + // Otherwise, the process needs to be continued until this method traverses the final inverted list + // and either generates some output in the output buffer or finishes traversing it. + if (isFinalList && doneMerge) { return doneMerge; } - - invListCursor.unloadPages(); - invListCursor.close(); } // Control does not reach here. @@ -195,8 +202,11 @@ public class InvertedListMerger { if (doneMerge) { // Final calculation is done. - finalInvListCursor.unloadPages(); - finalInvListCursor.close(); + try { + finalInvListCursor.unloadPages(); + } finally { + finalInvListCursor.close(); + } } return doneMerge; @@ -553,8 +563,14 @@ public class InvertedListMerger { * Cleans every stuff. */ public void close() throws HyracksDataException { - prevSearchResult.close(); - newSearchResult.close(); + try { + prevSearchResult.close(); + newSearchResult.close(); + } finally { + if (finalInvListCursor != null) { + finalInvListCursor.close(); + } + } } // Resets the variables.