Murtadha Hubail has posted comments on this change. Change subject: Cleanup Buffer Cache ......................................................................
Patch Set 1: (17 comments) https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: PS1, Line 65: synchronized is this synchronized needed anymore? PS1, Line 75: synchronized is this synchronized needed anymore? https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java: PS1, Line 111: 75 Unused PS1, Line 114: 78 Unused https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: PS1, Line 48: HyracksDataException Error code https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties File hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties: PS1, Line 89: element elements PS1, Line 90: is was or is not active https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/IOManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/io/IOManager.java: PS1, Line 54: openFileCount I believe this was for debugging. If so, please remove it. https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/build/IndexBuilder.java File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/build/IndexBuilder.java: PS1, Line 85: index.destroy(); It would be nice to add a log message here before destroying. I think it will save us a lot of time one day. https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: PS1, Line 83: @throws IOException fix docs https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: Line 42: Java doc https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndex.java: PS1, Line 742: Unsupported Error code or throw UnsupportedOperationException https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndex.java File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/ondisk/OnDiskInvertedIndex.java: PS1, Line 640: can error code https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: PS1, Line 799: Exception put a comment here to why catching Exception is needed https://asterix-gerrit.ics.uci.edu/#/c/1840/1/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientFileMapManager.java File hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/file/TransientFileMapManager.java: PS1, Line 31: Map How about changing to ConcurrentHashMap similar to Asterix FileMapManager and removing the sync? Just make them look the same if possible :) https://asterix-gerrit.ics.uci.edu/#/c/1840/1/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: PS1, Line 105: WS 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: 1 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-HasComments: Yes
