asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ian Maxon (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: Implemented LSM disk components alignment
Date Tue, 09 May 2017 00:06:45 GMT
Ian Maxon has posted comments on this change.

Change subject: Implemented LSM disk components alignment
......................................................................


Patch Set 7:

(5 comments)

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

PS7, Line 332: needsAlignment
> What about 'isCorrelated'?
Yeah, that's better.


PS7, Line 337: getPartitionID
> I carried 'partition' info when constructing LSMIndex (which causes a lot o
I'm not sure that's quite right either. Why does the index need to know about the partition
it's in?


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

PS7, Line 59: waitingOperationGroups
> There should be no concurrent access to this variable (all accesses of this
Ah, yes that's true.


PS7, Line 97: offer
> Just to be consistent with the previous 'runningFlushOperations' (line 80).
I see. That's fine. I was thinking based on the interface rather than that implementation
of it.


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

PS7, Line 458: );
> If it is a flush operation, then ctx.getComponentHolder() would only contai
Ok, well, maybe a comment or something to make that more explicit would be nice, or maybe
something to check the size. That's what I sort of assumed it was doing but it's not super
clear that it's 0 because there shouldn't be any other memory component.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I64bf34e255def72adc73b9f87cfa628a172ea694
Gerrit-PatchSet: 7
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <cluo8@uci.edu>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Jianfeng Jia <jianfeng.jia@gmail.com>
Gerrit-Reviewer: Luo Chen <cluo8@uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message