abdullah alamoudi has posted comments on this change. Change subject: Cleanup Buffer Cache ......................................................................
Patch Set 8: (5 comments) https://asterix-gerrit.ics.uci.edu/#/c/1840/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexFileManager.java: PS6, Line 81: /** : * @return true if the index dir exists, false otherwise : */ : public boolean exists(); > I think that making this explicit and having the method should be fine, but Done https://asterix-gerrit.ics.uci.edu/#/c/1840/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/api/IInvertedIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/api/IInvertedIndex.java: PS6, Line 43: /** : * Purge the index files out of the buffer cache. : * Can only be called if the caller is absolutely sure the files don't contain dirty pages : * : * @throws HyracksDataException : * if the index is active : */ : void purge() throws HyracksDataException; > "purge" confuses me, not so much the content, but why it is on this interfa Sure. The main reason is that: 1. InvertedIndexes don't implement the ITreeIndex interface. 2. when we deactivate a disk component, we need to purge it so the buffer cache doesn't go through each page. 3. look at line 181 of LSMInvertedIndex. we need to get the inverted index of a disk component and purge it. the call returns IInvertedIndex. I think the right way to handle this (and other diversions in LSMInvertedIndex is to revisit the class design) but I think that should be in a separate change https://asterix-gerrit.ics.uci.edu/#/c/1840/6/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/IFileMapManager.java: PS6, Line 27: IFileMapManager > While I think that we shouldn't add @author tags, we also shouldn't remove I agree. however, keeping them while changing the content also seems unfair to them. I will put it back... https://asterix-gerrit.ics.uci.edu/#/c/1840/8/hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-common-test/src/test/java/org/apache/hyracks/storage/common/BufferCacheTest.java File hyracks-fullstack/hyracks/hyracks-tests/hyracks-storage-common-test/src/test/java/org/apache/hyracks/storage/common/BufferCacheTest.java: PS8, Line 105: > WS Done PS8, Line 113: > WS Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1840 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15565b07afdc94ac74c608bfe4480fa09dcf8f1c Gerrit-PatchSet: 8 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
