[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116166622 ## File path: server/src/com/cloud/vm/UserVmManagerImpl.java ## @@ -3761,6 +3767,8 @@ public boolean setupVmForPvlan(boolean add, Long hostId, NicProfile nic) { } catch (AgentUnavailableException e) { s_logger.warn("Agent Unavailable ", e); return false; +} catch (OperationCancelledException e) { Review comment: it answer is null, false is returned at the end. removed return in the above exception handling. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116166347 ## File path: server/src/com/cloud/network/router/NetworkHelperImpl.java ## @@ -171,8 +172,10 @@ public boolean sendCommandsToRouter(final VirtualRouter router, final Commands c try { answers = _agentMgr.send(router.getHostId(), cmds); } catch (final OperationTimedoutException e) { -s_logger.warn("Timed Out", e); +s_logger.warn("Timed out", e); throw new AgentUnavailableException("Unable to send commands to virtual router ", router.getHostId(), e); +} catch (final OperationCancelledException e) { Review comment: added rethrow. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116166198 ## File path: server/src/com/cloud/ha/KVMFencer.java ## @@ -98,10 +99,7 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { FenceAnswer answer; try { answer = (FenceAnswer)_agentMgr.send(h.getId(), fence); -} catch (AgentUnavailableException e) { -s_logger.info("Moving on to the next host because " + h.toString() + " is unavailable"); -continue; -} catch (OperationTimedoutException e) { +} catch (AgentUnavailableException | OperationCancelledException | OperationTimedoutException e) { Review comment: changed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116166097 ## File path: server/src/com/cloud/api/query/vo/AsyncJobJoinVO.java ## @@ -202,6 +205,14 @@ public String getInstanceUuid() { return instanceUuid; } +public void setRelated(String related) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116165984 ## File path: plugins/hypervisors/xenserver/src/com/cloud/ha/XenServerFencer.java ## @@ -75,12 +76,7 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { continue; } answer = (FenceAnswer)ans; -} catch (AgentUnavailableException e) { -if (s_logger.isDebugEnabled()) { -s_logger.debug("Moving on to the next host because " + h.toString() + " is unavailable"); -} -continue; -} catch (OperationTimedoutException e) { +} catch (AgentUnavailableException | OperationCancelledException | OperationTimedoutException e) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116165915 ## File path: plugins/hypervisors/simulator/src/com/cloud/ha/SimulatorFencer.java ## @@ -93,7 +94,7 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { s_logger.debug("Moving on to the next host because " + h.toString() + " is unavailable"); } continue; -} catch (OperationTimedoutException e) { +} catch (OperationTimedoutException | OperationCancelledException e) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116165314 ## File path: plugins/hypervisors/ovm3/src/main/java/com/cloud/hypervisor/ovm3/resources/Ovm3FenceBuilder.java ## @@ -93,10 +94,9 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { FenceAnswer answer; try { answer = (FenceAnswer) agentMgr.send(h.getId(), fence); -} catch (AgentUnavailableException | OperationTimedoutException e) { +} catch (AgentUnavailableException | OperationTimedoutException |OperationCancelledException e) { if (LOGGER.isDebugEnabled()) { -LOGGER.debug("Moving on to the next host because " -+ h.toString() + " is unavailable", e); +LOGGER.debug("Moving on to the next host because " + h.toString() + " is unavailable", e); Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116165085 ## File path: plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmFencer.java ## @@ -102,6 +103,11 @@ public Boolean fenceOff(VirtualMachine vm, Host host) { s_logger.debug("Moving on to the next host because " + h.toString() + " is unavailable"); } continue; +} catch (OperationCancelledException e) { Review comment: yes. made the changes. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116164905 ## File path: plugins/hypervisors/hyperv/src/org/apache/cloudstack/storage/motion/HypervStorageMotionStrategy.java ## @@ -136,7 +137,7 @@ private Answer migrateVmWithVolumes(VMInstanceVO vm, VirtualMachineTO to, Host s } return answer; -} catch (OperationTimedoutException e) { +} catch (OperationTimedoutException | OperationCancelledException e) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116164729 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java ## @@ -1047,9 +1091,25 @@ public boolean start() { @Override public boolean stop() { +List jobs = _syncQueueItemDao.getActiveQueueItems(getMsid(), false); +for (SyncQueueItemVO job : jobs) { +AsyncJobVO childjob = _jobDao.findById(job.getContentId()); +if (childjob != null) { +Long parentjobid = Long.parseLong(childjob.getRelated()); Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116164374 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java ## @@ -762,12 +768,34 @@ protected void runInContext() { protected void reallyRun() { try { List l = _queueMgr.dequeueFromAny(getMsid(), MAX_ONETIME_SCHEDULE_SIZE); +boolean isPurged = false; if (l != null && l.size() > 0) { for (SyncQueueItemVO item : l) { -if (s_logger.isDebugEnabled()) { -s_logger.debug("Execute sync-queue item: " + item.toString()); +if (item.getContentType().equalsIgnoreCase(SyncQueueItem.AsyncJobContentType)) { +AsyncJob job = _jobDao.findById(item.getContentId()); +if (job != null && StringUtils.isNotBlank(job.getRelated())) { +AsyncJob parentJob = _jobDao.findById(Long.valueOf(job.getRelated())); +//If the parent job is done, do not execute the child. complete it and purge it +// from queue +if (parentJob != null && parentJob.getStatus().done() && !isPseudoJob(parentJob)) { +if (s_logger.isDebugEnabled()) { +s_logger.debug("Purging sync-queue item: " + item.toString()); +} +completeAsyncJob(item.getContentId(), parentJob.getStatus(), 0, "Job is not " ++ "scheduled for execution as the parent job is done. Parent Job " + +"state:" + " " + parentJob.getStatus()); + _jobMonitor.unregisterByJobId(item.getContentId()); +_queueMgr.purgeItem(item.getId()); +isPurged = true; +} +} +} +if (!isPurged) { Review comment: modified. see above comment. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116164295 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java ## @@ -762,12 +768,34 @@ protected void runInContext() { protected void reallyRun() { try { List l = _queueMgr.dequeueFromAny(getMsid(), MAX_ONETIME_SCHEDULE_SIZE); +boolean isPurged = false; if (l != null && l.size() > 0) { for (SyncQueueItemVO item : l) { -if (s_logger.isDebugEnabled()) { -s_logger.debug("Execute sync-queue item: " + item.toString()); +if (item.getContentType().equalsIgnoreCase(SyncQueueItem.AsyncJobContentType)) { Review comment: its just 3 method calls. i will leave it like this for now. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116164162 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java ## @@ -762,12 +768,34 @@ protected void runInContext() { protected void reallyRun() { try { List l = _queueMgr.dequeueFromAny(getMsid(), MAX_ONETIME_SCHEDULE_SIZE); +boolean isPurged = false; Review comment: It will run again in the next 2 seconds. I dont remember why its out of for loop. may be there were some issues with the dequeue. I move it in and see. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116158218 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/AsyncJobManager.java ## @@ -131,4 +131,6 @@ void joinJob(long jobId, long joinJobId, String wakeupHandler, String wakupDispa Object unmarshallResultObject(AsyncJob job); List findFailureAsyncJobs(String... cmds); + Review comment: removed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116158204 ## File path: engine/orchestration/src/com/cloud/agent/manager/AgentManagerImpl.java ## @@ -1635,6 +1677,24 @@ protected void runInContext() { } } +protected class CancelTask extends ManagedContextRunnable { +@Override +protected void runInContext() { +final List jobs = _asyncJobDao.getCancelledJobs(); Review comment: it might fetch again. but, it wont process again as they wont be in the sequence map. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151896 ## File path: api/src/com/cloud/exception/OperationTimedoutException.java ## @@ -67,4 +68,12 @@ public int getWaitTime() { public boolean isActive() { return _isActive; } + +public boolean isCancelled() { +return _isCancelled; +} + +public void setCancelled(boolean isCancelled) { Review comment: removed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151902 ## File path: api/src/org/apache/cloudstack/api/command/user/job/CancelAsyncJobCmd.java ## @@ -0,0 +1,95 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.job; + +import com.cloud.event.EventTypes; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.user.Account; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiCommandJobType; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.vm.DestroyVMCmd; +import org.apache.cloudstack.api.response.AsyncJobResponse; +import org.apache.cloudstack.jobs.AsyncJobService; +import org.apache.log4j.Logger; +import com.google.common.base.Strings; + +import javax.inject.Inject; + +@APICommand(name = "cancelAsyncJob", description = "Cancel asynchronous job.", +responseObject = AsyncJobResponse.class, +requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.7.1", authorized = {RoleType.Admin}) Review comment: updated This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151889 ## File path: api/src/com/cloud/exception/OperationTimedoutException.java ## @@ -67,4 +68,12 @@ public int getWaitTime() { public boolean isActive() { return _isActive; } + +public boolean isCancelled() { Review comment: removed This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151883 ## File path: api/src/com/cloud/exception/OperationTimedoutException.java ## @@ -38,6 +38,7 @@ // transient Command[] _cmds; boolean _isActive; +boolean _isCancelled; Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151868 ## File path: api/src/com/cloud/exception/AgentUnavailableException.java ## @@ -38,4 +40,18 @@ public AgentUnavailableException(long agentId) { public AgentUnavailableException(String msg, long agentId, Throwable cause) { super("Host " + agentId + ": " + msg, Host.class, agentId, cause); } + +public AgentUnavailableException(String msg, long agentId, boolean isCancelled) { +this(msg, agentId, null); +setIsCancelled(isCancelled); +} + +public boolean isCancelled() { +return isCancelled; +} + +public void setIsCancelled(boolean isCancelled) { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r116151873 ## File path: api/src/com/cloud/exception/OperationCancelledException.java ## @@ -0,0 +1,73 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.exception; + +import com.cloud.agent.api.Command; + +import java.util.Arrays; + +/** + * job can be cancelled using async job cancel api + */ +public class OperationCancelledException extends CloudException { Review comment: done This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115916534 ## File path: framework/jobs/src/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java ## @@ -1075,4 +1135,65 @@ private void publishOnEventBus(AsyncJob job, String jobEvent) { public List findFailureAsyncJobs(String... cmds) { return _jobDao.getFailureJobsSinceLastMsStart(getMsid(), cmds); } + +@Override +public String cancelAsyncJob(long jobId, String reason) { +final AsyncJobVO job = _jobDao.findById(jobId); +String errString; + +if (job == null) { +errString="Cannot cancel. job-" + jobId + " no longer exists."; +if (s_logger.isDebugEnabled()) { +s_logger.debug(errString); +} +// still purge item from queue to avoid any blocking +_queueMgr.purgeAsyncJobQueueItemId(jobId); +return errString; +} +try { +Class cmdClass = Class.forName(job.getCmd()); +if (! CancellableCmd.class.isAssignableFrom(cmdClass)) { +errString="Cannot cancel. job-" + jobId + " as it is not cancellable."; +if (s_logger.isDebugEnabled()) { +s_logger.debug(errString); +} +return errString; +} +}catch (ClassNotFoundException e){ +errString="Command- " + job.getCmd() + " of jobid- " + jobId + " not found."; +s_logger.error(errString, e); +return errString; +} +if (job.getStatus() == Status.IN_PROGRESS) { +if (s_logger.isDebugEnabled()) { +s_logger.debug("cancelling job-" + jobId + " which is in IN_PROGRESS state."); +} +try { +completeAsyncJob(jobId, JobInfo.Status.CANCELLED, 0, "Job is cancelled due to " + reason); +_jobMonitor.unregisterByJobId(jobId); + +// purge the item and resume queue processing +_queueMgr.purgeItem(jobId); + +return ""; Review comment: Can be anything based on the method contract. empty just avoids null points incase its not checked properly by the caller. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115915881 ## File path: engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java ## @@ -1107,6 +1110,13 @@ public void orchestrateStart(final String vmUuid, final Map
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115914969 ## File path: engine/orchestration/src/com/cloud/agent/manager/AgentAttache.java ## @@ -399,10 +414,22 @@ public void send(final Request req, final Listener listener) throws AgentUnavail try { for (int i = 0; i < 2; i++) { Answer[] answers = null; +job = _agentMgr._asyncJobDao.findById(jobId); +if (job != null && job.getStatus() == JobInfo.Status.CANCELLED) { +throw new OperationCancelledException(req.getCommands(), _id, seq, wait, false); +} try { answers = sl.waitFor(wait); +job = _agentMgr._asyncJobDao.findById(jobId); +if (job != null && job.getStatus() == JobInfo.Status.CANCELLED) { +throw new OperationCancelledException(req.getCommands(), _id, seq, wait, false); Review comment: I agree. will do the change. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115914866 ## File path: engine/components-api/src/com/cloud/agent/AgentManager.java ## @@ -80,9 +81,9 @@ *should the agent stop execution on the first error. * @return an array of Answer */ -Answer[] send(Long hostId, Commands cmds) throws AgentUnavailableException, OperationTimedoutException; +Answer[] send(Long hostId, Commands cmds) throws AgentUnavailableException, OperationTimedoutException, OperationCancelledException; -Answer[] send(Long hostId, Commands cmds, int timeout) throws AgentUnavailableException, OperationTimedoutException; +Answer[] send(Long hostId, Commands cmds, int timeout) throws AgentUnavailableException, OperationTimedoutException, OperationCancelledException; Review comment: OperationCancelledException is a checked exception. There will be compiler errors if its not thrown by the right methods. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115914811 ## File path: engine/api/src/com/cloud/vm/VirtualMachineManager.java ## @@ -108,7 +109,7 @@ void orchestrateStart(String vmUuid, Map pa void advanceStop(String vmUuid, boolean cleanupEvenIfUnableToStop) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException; -void advanceExpunge(String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException; +void advanceExpunge(String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException, OperationCancelledException; Review comment: OperationCancelledException is a checked exception. There will be compiler errors if its not thrown by the right methods. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115914294 ## File path: api/src/org/apache/cloudstack/api/command/user/job/CancelAsyncJobCmd.java ## @@ -0,0 +1,95 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.user.job; + +import com.cloud.event.EventTypes; +import com.cloud.exception.ResourceUnavailableException; +import com.cloud.user.Account; +import org.apache.cloudstack.acl.RoleType; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiCommandJobType; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.user.vm.DestroyVMCmd; +import org.apache.cloudstack.api.response.AsyncJobResponse; +import org.apache.cloudstack.jobs.AsyncJobService; +import org.apache.log4j.Logger; +import com.google.common.base.Strings; + +import javax.inject.Inject; + +@APICommand(name = "cancelAsyncJob", description = "Cancel asynchronous job.", +responseObject = AsyncJobResponse.class, +requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.7.1", authorized = {RoleType.Admin}) +public class CancelAsyncJobCmd extends BaseCmd { +public static final Logger s_logger = Logger.getLogger(DestroyVMCmd.class.getName()); + +private static final String s_name = "cancelasyncjobresponse"; + +@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = AsyncJobResponse.class, +required = true, description = "The ID of the job to cancel") +protected Long id; + +@Inject +private AsyncJobService asyncJobService; + + +public String getEventType() { +return EventTypes.EVENT_JOB_CANCEL; +} + +public String getEventDescription() { +return "cancelling job with id: " + id; +} + +@Override +public void execute() throws ResourceUnavailableException, ClassNotFoundException { +String errorString = asyncJobService.cancelAsyncJob(id, "cancel request by user using cancelAsyncJob api"); +if(Strings.isNullOrEmpty(errorString)) { Review comment: cancelasyncjob will not return a blank string. this check should be sufficient. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs
karuturi commented on a change in pull request #1832: CLOUDSTACK-9652 Job framework - Cancelling async jobs URL: https://github.com/apache/cloudstack/pull/1832#discussion_r115656830 ## File path: server/src/com/cloud/server/ManagementServerImpl.java ## @@ -2793,6 +2795,8 @@ public long getMemoryOrCpuCapacityByHost(final Long hostId, final short capacity cmdList.add(UpdateIsoCmd.class); cmdList.add(UpdateIsoPermissionsCmd.class); cmdList.add(ListAsyncJobsCmd.class); +cmdList.add(ListLongRunningAsyncJobsCmd.class); +cmdList.add(ListQueuedUpAsyncJobsCmd.class); Review comment: its in asyncjobmanagerimpl This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services