asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Murtadha Hubail (Code Review)" <>
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:

File hyracks-fullstack/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/

Line 31:      *
add missing param

Line 44:      *
add missing param
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/

PS8, Line 77:  
() ->
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/

Line 357:     }
> CRITICAL SonarQube violation:
replace those \ by % while you are at it
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/

Line 45: public class VirtualBufferCache implements IVirtualBufferCache {

PS8, Line 59: 

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: 

PS8, Line 197: 

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

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

PS8, Line 344:  

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

PS8, Line 445: 

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ae9736c9b5fdba5795245bdf835c023e3f73b15
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <>
Gerrit-Reviewer: Ian Maxon <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Luo Chen <>
Gerrit-Reviewer: Michael Blow <>
Gerrit-Reviewer: Murtadha Hubail <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: Xikui Wang <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message