Ian Maxon has submitted this change and it was merged. Change subject: Fixes for ASTERIXDB-1636 ......................................................................
Fixes for ASTERIXDB-1636 The index of the tuple field for filters from SecondaryIndexOperationsHelper and AqlMetadataProvider differed. The one in AqlMetadataProvider was wrong, as it was attempting to take into account the presence of a partitioning field in the incoming tuple, which is not there in the case of an insert/upsert. There was also an issue where on merge, for components with a filter page but no min/max, the merge would fail. I fixed this by skipping over null entries while getting the min/max of merging components. Finally, there was a very silly error in LSMComponentFilterManager which was causing the filter page to appear as blank, because the page was being pinned with the wrong argument. That is also fixed. Change-Id: Ib4bc413fcda9a5c98ae57f94e1c8a68fe9aacda3 Reviewed-on: https://asterix-gerrit.ics.uci.edu/1205 Sonar-Qube: Jenkins <[email protected]> Reviewed-by: Taewoo Kim <[email protected]> Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> --- M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java M hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java 3 files changed, 22 insertions(+), 19 deletions(-) Approvals: Taewoo Kim: Looks good to me, approved Jenkins: Verified; No violations found; Verified diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java index 29087ba..e05aa25 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/AqlMetadataProvider.java @@ -1833,7 +1833,6 @@ // One token (+ optional partitioning field) + primary keys: [token, // number of token, PK] int numKeys = primaryKeys.size() + secondaryKeys.size(); - int numTokenKeyPairFields = (!isPartitioned) ? 1 + primaryKeys.size() : 2 + primaryKeys.size(); int numFilterFields = DatasetUtils.getFilterField(dataset) == null ? 0 : 1; // generate field permutations @@ -1960,8 +1959,9 @@ } filterFieldsForNonBulkLoadOps = new int[numFilterFields]; - filterFieldsForNonBulkLoadOps[0] = numTokenKeyPairFields; - invertedIndexFieldsForNonBulkLoadOps = new int[numTokenKeyPairFields]; + //for non-bulk-loads, there is only <SK,PK,F> in the incoming tuples + filterFieldsForNonBulkLoadOps[0] = numKeys; + invertedIndexFieldsForNonBulkLoadOps = new int[numKeys]; for (int k = 0; k < invertedIndexFieldsForNonBulkLoadOps.length; k++) { invertedIndexFieldsForNonBulkLoadOps[k] = k; } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java index d0d7e69..93225cf 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java @@ -56,36 +56,33 @@ } @Override - public void writeFilterInfo(ILSMComponentFilter filter, ITreeIndex treeIndex ) throws HyracksDataException { + public void writeFilterInfo(ILSMComponentFilter filter, ITreeIndex treeIndex) throws HyracksDataException { IMetaDataPageManager treeMetaManager = treeIndex.getMetaManager(); ICachedPage filterPage = null; int componentFilterPageId = treeMetaManager.getFilterPageId(); boolean appendOnly = false; int fileId = treeIndex.getFileId(); - if(componentFilterPageId == LinkedMetaDataPageManager.NO_FILTER_IN_PLACE){//in-place mode, no filter page yet + if (componentFilterPageId == LinkedMetaDataPageManager.NO_FILTER_IN_PLACE) { //in-place mode, no filter page yet ITreeIndexMetaDataFrame metadataFrame = treeIndex.getMetaManager().getMetaDataFrameFactory().createFrame(); int metaPageId = treeMetaManager.getFirstMetadataPage(); ICachedPage metadataPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, metaPageId), false); metadataPage.acquireWriteLatch(); - try{ + try { metadataFrame.setPage(metadataPage); componentFilterPageId = treeIndex.getMetaManager().getFreePage(metadataFrame); metadataFrame.setLSMComponentFilterPageId(componentFilterPageId); - } - finally{ + } finally { metadataPage.releaseWriteLatch(true); bufferCache.unpin(metadataPage); } - } - else if (componentFilterPageId <= LinkedMetaDataPageManager.NO_FILTER_APPEND_ONLY){ + } else if (componentFilterPageId <= LinkedMetaDataPageManager.NO_FILTER_APPEND_ONLY) { appendOnly = true; filterPage = treeMetaManager.getFilterPage(); - if(filterPage == null){ + if (filterPage == null) { treeMetaManager.setFilterPage(bufferCache.confiscatePage(IBufferCache.INVALID_DPID)); filterPage = treeMetaManager.getFilterPage(); } - } - else{// in place, not a new filter page + } else { // in place, not a new filter page filterPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, componentFilterPageId), true); } @@ -102,11 +99,10 @@ } } finally { - if(!appendOnly){ + if (!appendOnly) { bufferCache.unpin(filterPage); filterPage.releaseWriteLatch(true); - } - else{ + } else { filterPage.releaseWriteLatch(false); } } @@ -122,7 +118,8 @@ if (componentFilterPageId < 0) return false; - ICachedPage filterPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, componentFilterPageId), true); + ICachedPage filterPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, componentFilterPageId), + false); filterPage.acquireReadLatch(); try { diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java index 204e874..bf6e9b5 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java @@ -645,8 +645,14 @@ if (component.getLSMComponentFilter() != null) { List<ITupleReference> filterTuples = new ArrayList<ITupleReference>(); for (int i = 0; i < mergeOp.getMergingComponents().size(); ++i) { - filterTuples.add(mergeOp.getMergingComponents().get(i).getLSMComponentFilter().getMinTuple()); - filterTuples.add(mergeOp.getMergingComponents().get(i).getLSMComponentFilter().getMaxTuple()); + ITupleReference min = mergeOp.getMergingComponents().get(i).getLSMComponentFilter().getMinTuple(); + ITupleReference max = mergeOp.getMergingComponents().get(i).getLSMComponentFilter().getMaxTuple(); + if (min != null) { + filterTuples.add(min); + } + if (max != null) { + filterTuples.add(max); + } } filterManager.updateFilterInfo(component.getLSMComponentFilter(), filterTuples); filterManager.writeFilterInfo(component.getLSMComponentFilter(), -- To view, visit https://asterix-gerrit.ics.uci.edu/1205 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib4bc413fcda9a5c98ae57f94e1c8a68fe9aacda3 Gerrit-PatchSet: 7 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]>
