Till Westmann has posted comments on this change.

Change subject: [ASTERIXDB-1911][HYR,RT,CLUS] Fixes and Improvements for 
PreDistributed Jobs
......................................................................


Patch Set 14:

(5 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2045/14/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java
File 
asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java:

Line 743:     public void handleCreateIndexStatement(MetadataProvider 
metadataProvider, Statement stmt,
> Used by BAD? Maybe we need a better solution here rather than simply change
Good point. How is this called in BAD?


https://asterix-gerrit.ics.uci.edu/#/c/2045/14/asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/LockThenSearchOperationCallbackFactory.java
File 
asterixdb/asterix-transactions/src/main/java/org/apache/asterix/transaction/management/opcallbacks/LockThenSearchOperationCallbackFactory.java:

Line 52:                     fact instanceof IJobEventListenerFactory ? 
((IJobEventListenerFactory) fact).getJobId() : jobId;
> I'm not sure this is the right way to get JobId. @Till needs to look at thi
It certainly seems "smelly" that 
a) we need an instanceof and that
b) we need to change the behavior in the middle of AsterixDB's transaction 
management when the job spec was deployed using different means.
Unfortunately, I don't understand the role of the IJobEventListenerFactory well 
enough to have a concrete proposal.


https://asterix-gerrit.ics.uci.edu/#/c/2045/14/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/IJobletEventListenerFactory.java
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/IJobletEventListenerFactory.java:

PS14, Line 31: updateListenerJobParameters
Why do we need to update? Can't this be done at construction?


https://asterix-gerrit.ics.uci.edu/#/c/2045/14/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/JobParameterByteStore.java
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/JobParameterByteStore.java:

PS14, Line 31: nonpureSingletonValues
What is a nonpure singleton value?


https://asterix-gerrit.ics.uci.edu/#/c/2045/14/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/PreDistributedId.java
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/PreDistributedId.java:

PS14, Line 30: PreDistributedId
I'm still confused by this name and I'm wondering if another name could help to 
gain more clarity. 
If I understand the lifecycle correctly, then it usually is
1) create a (parameterized) job spec
2) deploy the job spec to the cluster
3) start a job on the cluster (passing an identifier for the job spec and some 
parameters)
4) wait for the job to finish
Then we can repeat 3 and 4 as often as we like and finally 
5) remove the JobSpec from the cluster again.
If that's right, I'd propose the verbs "deploy"/"undeploy" to manage the 
existence of the JobSpec on the cluster (thereby treating the "cluster" as a 
single thing while "distribute" treats it as a collection of things) and to 
call the job spec a JobSpecification leading to the name
"DeployedJobSpecificationId" for this class. 

Does this argument and naming scheme make sense?

Also, it would be good to have a description of the lifecycle of deployed jobs 
in the "right" place in the javadocs. Is there a "right" place for these docs?


-- 
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: 14
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

Reply via email to