Michael Blow has posted comments on this change. Change subject: Big Object Support For Storage ......................................................................
Patch Set 2: (46 comments) Addressed comments in patch 2. https://asterix-gerrit.ics.uci.edu/#/c/840/2/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/ILogRecord.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/transactions/ILogRecord.java: Line 28: enum RECORD_STATUS { > Could we call this RecordReadStatus? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationManager.java File asterixdb/asterix-replication/src/main/java/org/apache/asterix/replication/management/ReplicationManager.java: Line 241: > empty line Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/api/IBTreeLeafFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/api/IBTreeLeafFrame.java: Line 62: void ensureCapacity(IMetaDataPageManager freePageManager, ITreeIndexMetaDataFrame metaFrame, IBufferCache bufferCache, > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeFieldPrefixNSMLeafFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeFieldPrefixNSMLeafFrame.java: Line 468: strBuilder.append("flagOff: " + smFlagOff + "\n"); > s/flagOff/smFlagOff/ Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeNSMInteriorFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeNSMInteriorFrame.java: Line 110: throw new IllegalStateException("Space required for record (" + tupleSize > Could/should this be another exception? This check is redundant; removed. Line 200: throw new IllegalStateException("Space required for record (" + tupleSize > Could/should this be another exception? This check is redundant; removed. Line 211: public void split(ITreeIndexFrame rightFrame, ITupleReference tuple, ISplitKey splitKey, IMetaDataPageManager freePageManager, ITreeIndexMetaDataFrame metaFrame, IBufferCache bufferCache) > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeNSMLeafFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/BTreeNSMLeafFrame.java: Line 47: protected static final int supplementalNumPagesOff = nextLeafOff + 4; // 20 > Are these right? Done Line 200: throw new IllegalStateException(); > Remove this check? Done Line 212: : buf.capacity(); > Can we materialize this value? Done Line 224: // 1. fewer than 2 tuples currently > fix comment Done Line 225: if (getTupleCount() < 2) { > return getTupleCount() < 2 ? ... : ... ? Done Line 231: else { > else should be on the previous line Done Line 346: assert getFreeSpaceOff() < buf.capacity(); > remove assert Done Line 359: setLargeFlag(true); > This is idempotent, can we remove the if? Done Line 372: configureLargePage(framePagesOld + deltaPages - 1, freePageManager.getFreePageBlock(metaFrame, framePagesOld + deltaPages - 1)); > line too long Done Line 393: > remove Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTree.java: Line 222: ICachedPage smPage = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, pageId), false, largePageHelper); > line too long Done Line 251: ICachedPage newLeftNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, newLeftId), true, largePageHelper); > line too long Done Line 432: ICachedPage rightNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rightPageId), true, largePageHelper); > line too long Done Line 549: ICachedPage rightNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rightPageId), true, largePageHelper); > line too long Done Line 558: ctx.interiorFrame.split(rightFrame, ctx.splitKey.getTuple(), ctx.splitKey, freePageManager, ctx.metaFrame, bufferCache); > line too long Done Line 1071: final int multiplier = (int) Math.ceil((double) (tupleSize + 4) / (bufferCache.getPageSize() - headerSize)); > line too long Done Line 1101: assert (newSpaceUsed - spaceUsed) <= spaceNeeded : "Expected to use " + spaceNeeded + " bytes, instead used " + (newSpaceUsed - spaceUsed) + " bytes."; > line too long Removed assertion. Line 1121: protected void propagateBulk(int level, List<ICachedPage> pagesToWrite) throws HyracksDataException, TreeIndexException { > line too long Done Line 1135: throw new IllegalStateException("Space required for record (" + tupleBytes > Could/should we throw another exception here? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeCountingSearchCursor.java: Line 122: ICachedPage nextLeaf = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, nextLeafPage), false, BTreeLargeFrameHelper.INSTANCE); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/BTreeRangeSearchCursor.java: Line 121: ICachedPage nextLeaf = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, nextLeafPage), false, BTreeLargeFrameHelper.INSTANCE); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/AbstractSlotManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/AbstractSlotManager.java: Line 35: assert (offset >= 0 && offset <= frame.getBuffer().limit() - 4); > Revert the file? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/FrameOpSpaceStatus.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/FrameOpSpaceStatus.java: Line 23: INSUFFICIENT_SPACE, SUFFICIENT_CONTIGUOUS_SPACE, SUFFICIENT_SPACE, SUFFICIENT_INPLACE_SPACE, EXPAND, TOO_LARGE > align those vertically Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/TreeIndexNSMFrame.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/TreeIndexNSMFrame.java: Line 42: protected static final int levelOff = totalFreeSpaceOff + 4; // 28 > 28/29?? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/LinkedMetaDataPageManager.java: Line 99: public void addFreePageBlock(ITreeIndexMetaDataFrame metaFrame, int startingPage, int count) throws HyracksDataException { > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/impls/AbstractTreeIndex.java: Line 127: ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), true, largePageHelper); > line too long Done Line 245: ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false, largePageHelper); > line too long Done Line 263: ICachedPage rootNode = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), false, largePageHelper); > line too long Done Line 393: ICachedPage newRoot = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, rootPage), true, largePageHelper); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/util/TreeIndexStatsGatherer.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/util/TreeIndexStatsGatherer.java: Line 56: BufferedFileHandle.getDiskPageId(fileId, pageId), false, leafFrame.getLargePageHelper()); > line too long when tab size set to 4, not too long. https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/impls/RTree.java File hyracks-fullstack/hyracks/hyracks-storage-am-rtree/src/main/java/org/apache/hyracks/storage/am/rtree/impls/RTree.java: Line 415: ctx.interiorFrame.split(rightFrame, tuple, ctx.splitKey, freePageManager, ctx.metaFrame, bufferCache); > line too long Done Line 423: ctx.leafFrame.split(rightFrame, tuple, ctx.splitKey, freePageManager, ctx.metaFrame, bufferCache); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/AsyncFIFOPageQueueManager.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/AsyncFIFOPageQueueManager.java: Line 143: continue; > Why do we continue here? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/BufferCache.java: Line 568: resizePage(cPage, totalPages); > Check budget before resizing. Done Line 571: ioManager.syncRead(fInfo.getFileHandle(), (long) helper.getSupplementalBlockPageId(cPage) * pageSize, cPage.buffer); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/CachedPage.java: Line 44: static final boolean DEBUG = false; > Should we remove this? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java: Line 148: if (cPage.isGoodVictim()) { > This should always be true. Can we remove the "if"? Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/FieldPrefixNSMTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/FieldPrefixNSMTest.java: Line 139: ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(btreeFileId, 0), false, BTreeLargeFrameHelper.INSTANCE); > line too long Done https://asterix-gerrit.ics.uci.edu/#/c/840/2/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageManagerTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-am-btree-test/src/test/java/org/apache/hyracks/storage/am/btree/StorageManagerTest.java: Line 93: ICachedPage page = bufferCache.pin(BufferedFileHandle.getDiskPageId(fileId, pageId), false, BTreeLargeFrameHelper.INSTANCE); > line too long Done -- To view, visit https://asterix-gerrit.ics.uci.edu/840 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie0a3cb855768dfd0cd1da4a5fc9f62aedebcbc5f Gerrit-PatchSet: 2 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Michael Blow <michael.b...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <michael.b...@couchbase.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
