Arik Hadas has uploaded a new change for review. Change subject: engine: re-run mechanism improvements ......................................................................
engine: re-run mechanism improvements - RunVmCommandBase#rerun method no longer invokes rerunInternal method within a different thread because there is no need for a different thread when it is called from the command itself. the code of rerunInternal method was moved to rerun method without creating a new thread to run it instead. when rerun method is invoked by VdsEventListener, its code should still run in a different thread so it is called from a new thread in VdsEventListener#rerun method. - adjust the rerun mechanism such that the logs & events would reflect the process better: * on re-run, log the results of the previous run before checking whether the maximum rerun attempts were exceeded, so that it would be logged every time. * add a missing log call after exceeding the maximum re-run attempts to log a message that indicates the command failed. * make sure that when a command that is being rerun is failed due to can-do-action check, the failure event will appear only once. Change-Id: I557636a94d74772635102c6a33dc923d91332a3e Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java 4 files changed, 30 insertions(+), 23 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/84/9184/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java index 36ff593..6cf9994 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java @@ -209,7 +209,7 @@ } @Override - protected void rerunInternal() { + public void rerun() { /** * make Vm property to null in order to refresh it from db. */ @@ -221,7 +221,7 @@ */ if (getVm() != null && getVm().getstatus() == VMStatus.Up) { setVdsDestinationId(null); - super.rerunInternal(); + super.rerun(); } else { /** * vm went down on the destination and source, migration failed. diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java index a02ec01..081f679 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java @@ -36,7 +36,7 @@ } @Override - protected void rerunInternal() { + public void rerun() { /** * In case we failed to migrate to that specific server, the VM should no longer be pending, and we * report failure, without an attempt to rerun diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java index 7f1ecba..c4e8ce8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java @@ -206,26 +206,19 @@ } } - /** - * This function called when vds failed to run vm. Vm will be run on another vds (if exist) that's why the function - * should be run at a new thread because of it will lock a new VDSM - */ @Override - public final void rerun() { - ThreadPoolUtil.execute(new Runnable() { - @Override - public void run() { - rerunInternal(); - } - }); - } - - protected void rerunInternal() { + public void rerun() { Guid vdsId = getDestinationVds() != null ? getDestinationVds().getId() : getCurrentVdsId(); decreasePendingVms(vdsId); setSucceeded(false); setVm(null); + + // by default, if rerun is called then rerun process is about to start so log the result of the + //previous run as if rerun is about to begin (and change it later in case rerun isn't going to happen) + _isRerun = true; + log(); + /** * Rerun VM only if not exceeded maximum rerun attempts. for example if there are 10 hosts that can run VM and * predefine maximum 3 attempts to rerun VM - on 4th turn vm will stop to run despite there are still available @@ -233,10 +226,8 @@ */ if (getRunVdssList().size() < Config.<Integer> GetValue(ConfigValues.MaxRerunVmOnVdsCount) && getVm().getstatus() != VMStatus.Paused) { - _isRerun = true; // restore CanDoAction value to false so CanDoAction checks will run again getReturnValue().setCanDoAction(false); - log(); if (getExecutionContext() != null) { Job job = getExecutionContext().getJob(); if (job != null) { @@ -244,16 +235,25 @@ JobRepositoryFactory.getJobRepository().closeCompletedJobSteps(job.getId(), JobExecutionStatus.FAILED); } } + // set the _isRerun flag to false before calling executeAction to that we'll know if + // there is another rerun attempt within the method + _isRerun = false; executeAction(); - if (!getReturnValue().getCanDoAction()) { - _isRerun = false; + + // if there was no rerun attempt in the previous executeAction call and the command + // wasn't done because canDoAction check returned false.. + if (!_isRerun && !getReturnValue().getCanDoAction()) { log(); failedToRunVm(); } + + // signal the caller that a rerun was made + _isRerun = true; } else { Backend.getInstance().getResourceManager().RemoveAsyncRunningCommand(getVmId()); failedToRunVm(); _isRerun = false; + log(); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java index 4f7ff69..ff9922b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java @@ -331,9 +331,16 @@ @Override public void rerun(Guid vmId) { - IVdsAsyncCommand command = Backend.getInstance().getResourceManager().GetAsyncCommandForVm(vmId); + final IVdsAsyncCommand command = Backend.getInstance().getResourceManager().GetAsyncCommandForVm(vmId); if (command != null) { - command.rerun(); + // The command will be invoked in a different VDS in its rerun method, so we're calling + // its rerun method from a new thread so that it won't be executed within our current VDSM lock + ThreadPoolUtil.execute(new Runnable() { + @Override + public void run() { + command.rerun(); + } + }); } } -- To view, visit http://gerrit.ovirt.org/9184 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I557636a94d74772635102c6a33dc923d91332a3e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
