Yingyi Bu has posted comments on this change. Change subject: Cleanup Buffer Cache ......................................................................
Patch Set 6: (6 comments) https://asterix-gerrit.ics.uci.edu/#/c/1840/6//COMMIT_MSG Commit Message: PS6, Line 16: 7. open non existing file is not allowed. I think this should be allowed if WRITE is part of the mode. Otherwise, I worry about regressions. Currently even if io/txn/log dirs do not exist, an instance can still work fine. I guess to change that will add a lot of user burdens. https://asterix-gerrit.ics.uci.edu/#/c/1840/6/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/FileMapManager.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/FileMapManager.java: PS6, Line 47: fileId I wonder whether all methods have to be synchronized? It doesn't make sense to return a file id if the id has already been deleted from id2nameMap in unregister(...)? If that's the case, you don't need concurrent hash map for either. Ideally, a thread-safe BiMap can solve our problem. However, I couldn't find such an implementation. We might need to implement one at some point. https://asterix-gerrit.ics.uci.edu/#/c/1840/6/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/IoUtil.java: PS6, Line 30: public class IoUtil A brief doc for the class and each public method? https://asterix-gerrit.ics.uci.edu/#/c/1840/6/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/FileHandle.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/FileHandle.java: PS6, Line 55: READ_WRITE: : if (!fileRef.getFile().exists()) { : throw HyracksDataException.create(ErrorCode.FILE_DOES_NOT_EXIST, fileRef.getAbsolutePath()); : } Why only check file existence in READ_WRITE? BTW, I think auto creation for WRITE open() is nice, especially the directory. In this way, a user don't need to manually create io dirs etc. 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 don't think it is necessary if we auto-create dir if not exits. 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; Maybe I missed sth. Why can destory() take care of that internally? -- 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: 6 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: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
