Steven Jacobs has posted comments on this change. Change subject: Redeploy channels and procedures during recovery ......................................................................
Patch Set 5: (11 comments) 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 Procedures and Channels only allow one statement (so one result set) so this is a simplification for them. Maybe other deployed jobs will have potential issues, Line 249: if (useNewId) { > I'm comfused... What's the difference between deploy and redeploy + newId When the cluster is restarted, we just deploy the jobs as new jobs (the old deployed jobs would be lost anyway). When an index is added, we use the redeploy to replace the existing job spec 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 Done 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? Done 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 ? Done Line 109: public void storeExecutorService(ScheduledExecutorService ses) { > same Done 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? The return type was left over from functions, we don't use such a thing in procedures. I needed to add a Procedure type, so I figured I should remove the unused field while adding the new one. 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()? Done Line 112: for (IActiveEntityEventsListener listener : activeEventHandler.getEventListeners()) { > do you want to check & stop EventListeners' executorService before you remo Done Line 125: listener.suspend(); > this is to make sure if there is any dangling deployed job running instance yes 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. Sure. We can talk about it more in future, but the main issue is: 1) A repetitive procedure can exist without running (no one has called "execute" 2) At cluster restart, the listener is gone, so the only information we have is the metadata 3) For every channel we automatically know that there should be one running instance, so we just start it again. We have no such guarantee for a procedure, and the metadata doesn't tell us anything about if/how many were running. -- 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 <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Steven Jacobs <[email protected]> Gerrit-Reviewer: Xikui Wang <[email protected]> Gerrit-HasComments: Yes
