asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [NO ISSUE][STO] Fix memory leaks in storage
Date Fri, 03 Nov 2017 00:27:47 GMT
Murtadha Hubail has posted comments on this change.

Change subject: [NO ISSUE][STO] Fix memory leaks in storage
......................................................................


Patch Set 2:

(24 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2115/8/hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/DataUtils.java
File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/DataUtils.java:

Line 31:      *
add missing param


Line 44:      *
add missing param


https://asterix-gerrit.ics.uci.edu/#/c/2115/8/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MemoryComponentMetadata.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/MemoryComponentMetadata.java:

PS8, Line 77:  
() ->


https://asterix-gerrit.ics.uci.edu/#/c/2115/6/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java:

Line 357:     }
> CRITICAL SonarQube violation:
replace those \ by % while you are at it


https://asterix-gerrit.ics.uci.edu/#/c/2115/8/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/VirtualBufferCache.java:

Line 45: public class VirtualBufferCache implements IVirtualBufferCache {
@ThreadSafe


PS8, Line 59: 
pages


PS8, Line 63: che(ICacheMemoryAlloc
a comment saying what this is would be helpful. I know it's not your fault


PS8, Line 63: ic V
no need


PS8, Line 65: ;
if at any point we are exceeding this queue capacity by allocating an extra page, the freePages.offer(.)
will return false. Isn't this possible?


PS8, Line 65: ger(
no need


PS8, Line 124:        }
Remove this or make the logging level lower


PS8, Line 132:         
shouldn't this throw unsupportedOp?


PS8, Line 153:         --end;
This was for debugging, it isn't needed anymore


PS8, Line 163:  < end && firstUnused.dpid() != -
define a static method isLargePage and use it here and below. Possibly in ICachedPage


PS8, Line 187: VirtualPage page = null;
             :         int hash = hash(dpid);
             :         CacheBucket bucket = buckets[hash];
             : 
This was for debugging, it isn't needed anymore


PS8, Line 193: 
page


PS8, Line 197: 
!isLargePage


PS8, Line 199: }
Check the return value of offer if it is possible to exceed the fixed capacity of this queue.
If you are %100 sure that this won't happen, use add instead of offer, which throws an exception
if the capacity is exceeded. Otherwise, I would remove the capacity restriction.


PS8, Line 205:     
not needed


PS8, Line 306: g
synchronized


PS8, Line 308: (int i = 0; i < numPages; i++) {
This should be IllegalStateException


PS8, Line 344:  
synchronized


PS8, Line 346: ppend(String.format("Page size = %d\n", pageSize));
This should be IllegalStateException


PS8, Line 445: 
update


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2115
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae9736c9b5fdba5795245bdf835c023e3f73b15
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lychagin@couchbase.com>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Luo Chen <cluo8@uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhubail@apache.org>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message