Ian Maxon has posted comments on this change. Change subject: Make LSM bulkload append-only and write-once. ......................................................................
Patch Set 17: (12 comments) https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/freepage/LinkedMetaDataManager.java File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/freepage/LinkedMetaDataManager.java: Line 43: .getLogger("edu.uci.ics.hyracks.storage.am.common.freepage.LinkedMetaDataManager"); > replace string by LinkedMetaDataManager.class.getName() Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/AbstractTreeIndex.java File hyracks/hyracks-storage-am-common/src/main/java/edu/uci/ics/hyracks/storage/am/common/impls/AbstractTreeIndex.java: Line 115: //initEmptyTree(); > Why is this commented out instead of removing the line? Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-am-lsm-common/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java File hyracks/hyracks-storage-am-lsm-common/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/common/impls/LSMComponentFilterManager.java: Line 76: else if (componentFilterPageId < -1){//NO_FILTER_APPEND_ONLY){//append-only mode > Replace by == LinkedMetaDataManager.NO_FILTER_APPEND_ONLY Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java File hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/edu/uci/ics/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java: Line 819: //component.getInvIndex().create(); > let's remove this Done Line 900: } > This method is no-op: Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-am-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/rtree/impls/RTree.java File hyracks/hyracks-storage-am-rtree/src/main/java/edu/uci/ics/hyracks/storage/am/rtree/impls/RTree.java: Line 1038: //queue.offer(frontier.page); > should go away Done Line 1067: if (toRoot && !propagated && level < nodeFrontiers.size() - 1) { > checking propagated is useless since propagated is always false if toRoot i Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-common/src/main/java/edu/uci/ics/hyracks/storage/common/buffercache/CachedPage.java File hyracks/hyracks-storage-common/src/main/java/edu/uci/ics/hyracks/storage/common/buffercache/CachedPage.java: Line 48: virtual = new AtomicBoolean(); > just pass false to the constructor, and remove the next line. Done https://asterix-gerrit.ics.uci.edu/#/c/255/17/hyracks/hyracks-storage-common/src/main/java/edu/uci/ics/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java File hyracks/hyracks-storage-common/src/main/java/edu/uci/ics/hyracks/storage/common/buffercache/ClockPageReplacementStrategy.java: Line 41: this.maxAllowedNumPages = new AtomicInteger(maxAllowedNumPages); > why there are two maxAllowedNumPages? Both of them don't change the value i It used to change, but now it's probably fine to put back to a normal int. Line 81: if (maxAllowedNumPages.get() == 0) > When this value can be 0? It was just kind of asserted implicitly before, that it wouldn't be 0. We don't invoke this behavior any longer, so maybe an assert is better here. Line 102: clockPtr.set(clockPtr.incrementAndGet() % (numPages.get()-1)); > what is this -1 for? This is also cruft. Use to be, that the number of pages would thrash a lot, and it could be off by one. Line 118: } > When is addPage used instead of allocatePage()? It's actually cruft. We used to try fooling the eviction algorithm into trying to consider only non-confiscated pages, but it's very slow. -- To view, visit https://asterix-gerrit.ics.uci.edu/255 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I80fb891b5310252143854a336b591bf3f8cd4ba7 Gerrit-PatchSet: 17 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Ian Maxon <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Young-Seok Kim <[email protected]> Gerrit-HasComments: Yes
