Steven Jacobs has posted comments on this change. Change subject: [HYR,RT,CLUS] Fixes and Improvements for predistributed Jobs ......................................................................
Patch Set 6: (7 comments) 1. If we are replacing JobId here, should we call this new Id "predistributedId" as it doesn't only affect p-job but all jobs? The code paths exercised here don't affect jobs other than predistributed jobs. Normal jobs will never have predistributed ids. 2. we'd have to make sure this works for query cancellation/recovery/active runtime etc.. I think you are thinking that these changes affect regular jobs, but they don't. Only predistributed jobs will use the new code paths. https://asterix-gerrit.ics.uci.edu/#/c/2045/6//COMMIT_MSG Commit Message: Line 12: Allow predistributed jobs to have a new jobId for each execution > I didn't see you touch the transcation Id in this patch. When I did my fix, Not sure exactly what you mean here. Maybe we can talk on Skype about this? https://asterix-gerrit.ics.uci.edu/#/c/2045/6/asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java File asterixdb/asterix-algebra/src/main/java/org/apache/asterix/optimizer/rules/ConstantFoldingRule.java: Line 192: if (expr.getFunctionIdentifier().equals(BuiltinFunctions.IS_MISSING)) { > +1. I don't really see the reason why this special case is needed. Could yo Done https://asterix-gerrit.ics.uci.edu/#/c/2045/6/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/ActivityClusterGraph.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/ActivityClusterGraph.java: Line 35: > This seems to be not needed. Done https://asterix-gerrit.ics.uci.edu/#/c/2045/6/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: Line 56: private long predistributedId = 0; > I don't think to have a counter and do auto-increment here is a good idea. What is the disadvantage of doing it this way? https://asterix-gerrit.ics.uci.edu/#/c/2045/6/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: Line 44: node.getNodeController().destroyJob(predestributedId); > Are we certain that this wouldn't cause problems for query cancellation/act Right now there is no recovery procedure for predestributed jobs at all. This change makes incremental improvements without introducing new issues, but there is still future work to be done here. https://asterix-gerrit.ics.uci.edu/#/c/2045/6/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/CCNCFunctions.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/ipc/CCNCFunctions.java: Line 834: for (int i = 0; i < runTimeVarsSize; i++) { > ->runtimeVars. I think runtime is one word. Done https://asterix-gerrit.ics.uci.edu/#/c/2045/6/hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/PredistributedJobsTest.java File hyracks-fullstack/hyracks/hyracks-examples/hyracks-integration-tests/src/test/java/org/apache/hyracks/tests/integration/PredistributedJobsTest.java: Line 49: public class PredistributedJobsTest { > It would be nice to have a test case which can be a very simple query like Agreed, but this change doesn't actually allow for concurrent execution yet, it's just a step in the right direction. There is a Jira issue I have open for getting concurrency working. -- To view, visit https://asterix-gerrit.ics.uci.edu/2045 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8f493c1fa977d07dfe8a875f9ebe9515d01d1473 Gerrit-PatchSet: 6 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Ildar Absalyamov <ildar.absalya...@gmail.com> 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: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
