Xikui Wang has posted comments on this change.

Change subject: Redeploy channels and procedures during recovery
......................................................................


Patch Set 5:

(11 comments)

Testcases would be very much appreciated...

https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/BADJobService.java
File asterix-bad/src/main/java/org/apache/asterix/bad/BADJobService.java:

Line 142:             ResultReader resultReader = new ResultReader(hdc, jobId, 
new ResultSetId(0));
is this always reading the 1st result set? (I know there are some issues in a 
deployed job with multiple results. is this the one?)


Line 249:         if (useNewId) {
I'm comfused... What's the difference between deploy and redeploy + newId


https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateChannelStatement.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateChannelStatement.java:

Line 273:         resultsTableName = !push ? channelName + 
BADConstants.resultsEnding : "";
push : "" : constant


https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateProcedureStatement.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/lang/statement/CreateProcedureStatement.java:

PS5, Line 274: LANGUAGE_AQL
it's AQL?


https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/metadata/DeployedJobSpecEventListener.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/metadata/DeployedJobSpecEventListener.java:

Line 105:     public void storeDeployedJobSpecId(DeployedJobSpecId 
deployedJobSpecId) {
-> setDeployedJobSpecId ?


Line 109:     public void storeExecutorService(ScheduledExecutorService ses) {
same


https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/metadata/Procedure.java
File asterix-bad/src/main/java/org/apache/asterix/bad/metadata/Procedure.java:

Line 49:     public Procedure(String dataverseName, String functionName, int 
arity, List<String> params, String type,
Is it no longer the return type?


https://asterix-gerrit.ics.uci.edu/#/c/2641/5/asterix-bad/src/main/java/org/apache/asterix/bad/recovery/BADGlobalRecoveryManager.java
File 
asterix-bad/src/main/java/org/apache/asterix/bad/recovery/BADGlobalRecoveryManager.java:

PS5, Line 92: tionContext mdTxnCtx = 
MetadataManager.INSTANCE.beginTransaction();
            :         metadataProvider.setMetadataTxnContext(mdTxnCtx);
            : 
            :         List<Channel> channels = 
BADLangExtension.getAllChannels(mdTxnCtx);
            :         List<Procedure> procedures = 
BADLangExtension.getAllProcedures(mdTxnCtx);
            : 
            :         MetadataManager.INSTANCE.commitTransaction(mdTxnCtx);
why not do this in recover after doRecovery()?


Line 112:         for (IActiveEntityEventsListener listener : 
activeEventHandler.getEventListeners()) {
do you want to check & stop EventListeners' executorService before you remove 
them?


Line 125:             listener.suspend();
this is to make sure if there is any dangling deployed job running instance, 
they have to stop first?


PS5, Line 161:             //Issue: need to store in metadata the information 
for running instances
I don't understand this issue here... maybe we can talk about this offline...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6897ccf9cddb9ec8d10256e252ee893afe6db145
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb-bad
Gerrit-Branch: master
Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Xikui Wang <xkk...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to