Liron Aravot has posted comments on this change. Change subject: engine : placeholders of child commands aren't cleared on exception ......................................................................
Patch Set 3: Code-Review-1 (2 comments) http://gerrit.ovirt.org/#/c/31037/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java: Line 355: return getReturnValue(); Line 356: } Line 357: Line 358: private void clearChildAsyncTasksWithOutVdsmId() { Line 359: for (Entry<Guid, CommandBase<?>> entry : childCommandsMap.entrySet()) { I think that as part of this patch or in a further patch we should add logging to indicate what are the child command id's executed from the parent command or to save the parent id for the child commands tasks, otherwise it's impossible to track the flow. Line 360: entry.getValue().clearAsyncTasksWithOutVdsmId(); Line 361: entry.getValue().clearChildAsyncTasksWithOutVdsmId(); Line 362: } Line 363: } Line 1156: } catch (RuntimeException e) { Line 1157: processExceptionToClient(new VdcFault(e, VdcBllErrors.ENGINE)); Line 1158: log.error(String.format("Command %1$s throw exception", getClass().getName()), e); Line 1159: } finally { Line 1160: if (exceptionOccurred) { - what's the reason for having this code here opposing to the code that clears the tasks on executeAction()? any reason to not have both on the same place? (here or there) - this solution is problematic at some cases, the condition should be the same as in line 1164 (Exception may not be thrown). Line 1161: clearChildAsyncTasksWithOutVdsmId(); Line 1162: } Line 1163: // If we failed to execute due to exception or some other reason, we compensate for the failure. Line 1164: if (exceptionOccurred || !getSucceeded()) { -- To view, visit http://gerrit.ovirt.org/31037 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I457a1f9357fda9232a8757984d6936d9c977aa62 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
