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 <sjaco...@ucr.edu> Gerrit-Reviewer: Ian Maxon <ima...@apache.org> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Xikui Wang <xkk...@gmail.com> Gerrit-Reviewer: Yingyi Bu <buyin...@gmail.com> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
