asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Cleanup Buffer Cache
Date Thu, 15 Jun 2017 16:55:11 GMT
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 <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Murtadha Hubail <hubailmor@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message