Ian Maxon has submitted this change and it was merged. Change subject: Fixes related to ASTERIXDB-1534 ......................................................................
Fixes related to ASTERIXDB-1534 - Cleanup boolean conditions and exceptions from SonarQube comments - Fix issue where filter page in on-disk LSM components can be confused with root page on restart Change-Id: If51e0cd183f9d5ed6edaebef4a0568a6c67062e3 Reviewed-on: https://asterix-gerrit.ics.uci.edu/1111 Sonar-Qube: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Integration-Tests: Jenkins <[email protected]> Reviewed-by: Jianfeng Jia <[email protected]> Reviewed-by: Michael Blow <[email protected]> --- M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java M hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java 2 files changed, 38 insertions(+), 35 deletions(-) Approvals: Michael Blow: Looks good to me, approved Jianfeng Jia: Looks good to me, but someone else must approve Jenkins: Verified; No violations found; Verified diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java index fbb16b2..e7a1123 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java @@ -18,7 +18,6 @@ */ package org.apache.hyracks.storage.am.common.freepage; -import java.util.logging.Logger; import org.apache.hyracks.api.exceptions.HyracksDataException; import org.apache.hyracks.storage.am.common.api.IMetaDataPageManager; @@ -44,7 +43,6 @@ private boolean appendOnly = false; ICachedPage confiscatedMetaNode; ICachedPage filterPage; - private static Logger LOGGER = Logger.getLogger(LinkedMetaDataPageManager.class.getName()); public LinkedMetaDataPageManager(IBufferCache bufferCache, ITreeIndexMetaDataFrameFactory metaDataFrameFactory) { this.bufferCache = bufferCache; @@ -65,7 +63,8 @@ // allocate a new page in the chain of meta pages int newPage = metaFrame.getFreePage(); if (newPage < 0) { - throw new Exception("Inconsistent Meta Page State. It has no space, but it also has no entries."); + throw new HyracksDataException( + "Inconsistent Meta Page State. It has no space, but it also has no entries."); } ICachedPage newNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, newPage), false); @@ -87,8 +86,6 @@ bufferCache.unpin(newNode); } } - } catch (Exception e) { - e.printStackTrace(); } finally { metaNode.releaseWriteLatch(true); bufferCache.unpin(metaNode); @@ -181,7 +178,7 @@ @Override public int getMaxPage(ITreeIndexMetaDataFrame metaFrame) throws HyracksDataException { ICachedPage metaNode; - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { int mdPage = getFirstMetadataPage(); if (mdPage < 0) { return IBufferCache.INVALID_PAGEID; @@ -197,7 +194,7 @@ maxPage = metaFrame.getMaxPage(); } finally { metaNode.releaseReadLatch(); - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { bufferCache.unpin(metaNode); } } @@ -235,7 +232,7 @@ public int getFilterPageId() throws HyracksDataException { ICachedPage metaNode; int filterPageId = NO_FILTER_IN_PLACE; - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false); } else { metaNode = confiscatedMetaNode; @@ -251,7 +248,7 @@ } } finally { metaNode.releaseReadLatch(); - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { bufferCache.unpin(metaNode); } } @@ -316,7 +313,7 @@ @Override public boolean isMetaPage(ITreeIndexMetaDataFrame metaFrame) { - return (metaFrame.getLevel() == META_PAGE_LEVEL_INDICATOR); + return metaFrame.getLevel() == META_PAGE_LEVEL_INDICATOR; } @Override @@ -345,6 +342,7 @@ ITreeIndexMetaDataFrame metaFrame = metaDataFrameFactory.createFrame(); metaFrame.setPage(confiscatedMetaNode); int finalFilterPage = getFreePage(metaFrame); + setFilterPageId(finalFilterPage); bufferCache.setPageDiskId(filterPage, BufferedFileHandle.getDiskPageId(fileId, finalFilterPage)); queue.put(filterPage); } @@ -413,7 +411,7 @@ @Override public long getLSN() throws HyracksDataException { ICachedPage metaNode; - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { metaNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, getFirstMetadataPage()), false); } else { metaNode = confiscatedMetaNode; @@ -425,7 +423,7 @@ return metaFrame.getLSN(); } finally { metaNode.releaseReadLatch(); - if (!appendOnly || (appendOnly && confiscatedMetaNode == null)) { + if (!appendOnly || confiscatedMetaNode == null) { bufferCache.unpin(metaNode); } } diff --git a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java index 3a7eb77..0dcfc90 100644 --- a/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java +++ b/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java @@ -46,6 +46,7 @@ public abstract class AbstractTreeIndex implements ITreeIndex { public static final int MINIMAL_TREE_PAGE_COUNT = 2; + public static final int MINIMAL_TREE_PAGE_COUNT_WITH_FILTER = 3; protected int rootPage = 1; protected final IBufferCache bufferCache; @@ -70,12 +71,10 @@ protected int bulkloadLeafStart = 0; - public AbstractTreeIndex(IBufferCache bufferCache, IFileMapProvider fileMapProvider, - IMetaDataPageManager freePageManager, ITreeIndexFrameFactory interiorFrameFactory, - ITreeIndexFrameFactory leafFrameFactory, IBinaryComparatorFactory[] cmpFactories, - int fieldCount, - FileReference file) { + IMetaDataPageManager freePageManager, ITreeIndexFrameFactory interiorFrameFactory, + ITreeIndexFrameFactory leafFrameFactory, IBinaryComparatorFactory[] cmpFactories, int fieldCount, + FileReference file) { this.bufferCache = bufferCache; this.fileMapProvider = fileMapProvider; this.freePageManager = freePageManager; @@ -141,16 +140,25 @@ } } - private void setRootAndMetadataPages(boolean appendOnly) throws HyracksDataException{ + private void setRootAndMetadataPages(boolean appendOnly) throws HyracksDataException { if (!appendOnly) { // regular or empty tree rootPage = 1; bulkloadLeafStart = 2; } else { - // bulkload-only tree (used e.g. for HDFS). -1 is meta page, -2 is root page + //the root page is either page n-2 (no filter) or n-3 (filter) int numPages = bufferCache.getNumPagesOfFile(fileId); - //the root page is the last page before the metadata page - rootPage = numPages > MINIMAL_TREE_PAGE_COUNT ? numPages - MINIMAL_TREE_PAGE_COUNT : 0; + if (numPages > MINIMAL_TREE_PAGE_COUNT) { + int filterPageId = freePageManager.getFilterPageId(); + if (filterPageId > 0) { + rootPage = numPages - MINIMAL_TREE_PAGE_COUNT_WITH_FILTER; + } else { + rootPage = numPages - MINIMAL_TREE_PAGE_COUNT; + } + } else { + rootPage = 0; + } + //leaves start from the very beginning of the file. bulkloadLeafStart = 0; } @@ -188,10 +196,9 @@ int mdPageLoc = freePageManager.getFirstMetadataPage(); ITreeIndexMetaDataFrame metaFrame = freePageManager.getMetaDataFrameFactory().createFrame(); int numPages = freePageManager.getMaxPage(metaFrame); - if(mdPageLoc > 1 || (mdPageLoc == 1 && numPages <= MINIMAL_TREE_PAGE_COUNT -1)) { //md page doesn't count itself + if (mdPageLoc > 1 || (mdPageLoc == 1 && numPages <= MINIMAL_TREE_PAGE_COUNT - 1)) { //md page doesn't count appendOnly = true; - } - else{ + } else { appendOnly = false; } setRootAndMetadataPages(appendOnly); @@ -245,7 +252,7 @@ if (rootPage == -1) { return true; } - if(freePageManager.appendOnlyMode() && bufferCache.getNumPagesOfFile(fileId) <= MINIMAL_TREE_PAGE_COUNT){ + if (freePageManager.appendOnlyMode() && bufferCache.getNumPagesOfFile(fileId) <= MINIMAL_TREE_PAGE_COUNT) { return true; } ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false); @@ -262,8 +269,6 @@ bufferCache.unpin(rootNode); } } - - public byte getTreeHeight(ITreeIndexFrame frame) throws HyracksDataException { ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false); @@ -331,8 +336,8 @@ protected final IFIFOPageQueue queue; protected List<ICachedPage> pagesToWrite; - public AbstractTreeIndexBulkLoader(float fillFactor, boolean appendOnly) throws TreeIndexException, - HyracksDataException { + public AbstractTreeIndexBulkLoader(float fillFactor, boolean appendOnly) + throws TreeIndexException, HyracksDataException { //Initialize the tree if (appendOnly) { create(appendOnly); @@ -359,8 +364,8 @@ NodeFrontier leafFrontier = new NodeFrontier(leafFrame.createTupleReference()); leafFrontier.pageId = freePageManager.getFreePage(metaFrame); - leafFrontier.page = bufferCache.confiscatePage( - BufferedFileHandle.getDiskPageId(fileId, leafFrontier.pageId)); + leafFrontier.page = bufferCache + .confiscatePage(BufferedFileHandle.getDiskPageId(fileId, leafFrontier.pageId)); interiorFrame.setPage(leafFrontier.page); interiorFrame.initBuffer((byte) 0); @@ -382,10 +387,10 @@ for (NodeFrontier nodeFrontier : nodeFrontiers) { ICachedPage frontierPage = nodeFrontier.page; if (frontierPage.confiscated()) { - bufferCache.returnPage(frontierPage,false); + bufferCache.returnPage(frontierPage, false); } } - for(ICachedPage pageToDiscard: pagesToWrite){ + for (ICachedPage pageToDiscard : pagesToWrite) { bufferCache.returnPage(pageToDiscard, false); } releasedLatches = true; @@ -400,8 +405,8 @@ newRoot.acquireWriteLatch(); //root will be the highest frontier NodeFrontier lastNodeFrontier = nodeFrontiers.get(nodeFrontiers.size() - 1); - ICachedPage oldRoot = bufferCache.pin( - BufferedFileHandle.getDiskPageId(fileId, lastNodeFrontier.pageId), false); + ICachedPage oldRoot = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, lastNodeFrontier.pageId), + false); oldRoot.acquireReadLatch(); lastNodeFrontier.page = oldRoot; try { -- To view, visit https://asterix-gerrit.ics.uci.edu/1111 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: merged Gerrit-Change-Id: If51e0cd183f9d5ed6edaebef4a0568a6c67062e3 Gerrit-PatchSet: 3 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: Michael Blow <[email protected]>
