abdullah alamoudi has posted comments on this change. Change subject: Cleanup Buffer Cache ......................................................................
Patch Set 2: (15 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? yes because we need register and unregister to be mutually exclusive. and we're updating the two maps. PS1, Line 75: synchronized > is this synchronized needed anymore? yes because we need register and unregister to be mutually exclusive. and we're updating the two maps. 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: R > Unused Done PS1, Line 114: ; > Unused Done 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: cksDataException.cre > Error code Done 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 Done PS1, Line 90: is > was or is not active Done 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: > I believe this was for debugging. If so, please remove it. Done 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: //2. Node leaves > It would be nice to add a log message here before destroying. I think it wi Done 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: > fix docs Done 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 Done 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: ception(); > Error code or throw UnsupportedOperationException Done. This should be gone with a small refactoring of the inverted indexes 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: > error code Done 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 Done 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: Con > How about changing to ConcurrentHashMap similar to Asterix FileMapManager a 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: 2 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
