asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Steven Jacobs (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: ASTERIXDB-1747 Implemented full lifecycle capabilities for d...
Date Mon, 06 Feb 2017 22:32:08 GMT
Steven Jacobs has posted comments on this change.

Change subject: ASTERIXDB-1747 Implemented full lifecycle capabilities for distributed jobs
......................................................................


Patch Set 10:

(6 comments)

Addressed comments. As far as tests go, the functionality is tested in BAD, which is run against
all changes to Asterix, which should prevent regression.

https://asterix-gerrit.ics.uci.edu/#/c/1377/7/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClientInterfaceIPCI.java
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClientInterfaceIPCI.java:

PS7, Line 109: else
> formatting?
Done


https://asterix-gerrit.ics.uci.edu/#/c/1377/10/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java:

PS10, Line 359: removeActivityClusterGraphConstraints
> This seems to be used. Do we leak constraints here?
Good catch! FIXED!


https://asterix-gerrit.ics.uci.edu/#/c/1377/10/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/executor/JobExecutor.java
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/executor/JobExecutor.java:

PS10, Line 495: JavaSerializationUtils
> Do we serialize this for every TaskAttempt? Why do we need to do that?
Previously this was serialized for every invocation of StartTasks. Now it is only serialized
if the job is not predistributed. I also added the check for changed, since I noticed that
it's not actually used unless changed is true.


https://asterix-gerrit.ics.uci.edu/#/c/1377/10/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/JobRun.java
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/JobRun.java:

PS10, Line 67: ActivityClusterGraph
> Why is this not final anymore?
Made it final


PS10, Line 69: scheduler
> Why is this not final anymore?
This is created within the two public constructors, which call the private constructor. Since
it isn't assigned in the private constructor, it can't be final.


https://asterix-gerrit.ics.uci.edu/#/c/1377/10/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/DestroyJobWork.java
File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/DestroyJobWork.java:

PS10, Line 49: removeJobSpecification
> constraints are not removed
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I59c3422d5c1ab7756a6a4685ac527dfe50434954
Gerrit-PatchSet: 10
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Steven Jacobs <sjaco002@ucr.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Xikui Wang <xkkwww@gmail.com>
Gerrit-Reviewer: Yingyi Bu <buyingyi@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message