Till Westmann 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(); > Auto creating the dir is bad! I think that making this explicit and having the method should be fine, but I also think that it should be aligned with the other lifecycle methods createDirs and deleteDirs. I.e. it should be close to the other methods and the name should indicate the relationship (e.g. "dirsExist"). 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() has nothing to do with destry(). it is a quick way to tell the buff "purge" confuses me, not so much the content, but why it is on this interface. If it's a lifecycle method for indexes, then it probably should be on IIndex. On the other hand the implementation in LSMInvertedIndex throws UnsupportedOperationException, so it doesn't seem to be called for that index and thus it seems to be not applicable. Could you bring light to the mystery? 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 them, as the authorship doesn't change. Ideally the author himself would remove them, but otherwise they should remain in the file. https://asterix-gerrit.ics.uci.edu/#/c/1840/6/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: PS6, Line 105: WS PS6, Line 113: WS -- 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
