Ravi Nori has posted comments on this change. Change subject: engine : Persist Async Task root command in command_entities ......................................................................
Patch Set 6: (12 comments) http://gerrit.ovirt.org/#/c/31737/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskFactory.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/AsyncTaskFactory.java: Line 34: creationInfo.getStepId(), Line 35: creationInfo.getStoragePoolID(), Line 36: creationInfo.getTaskType(), Line 37: getCommandEntity(coco, Line 38: asyncTask == null ? Guid.newGuid() : asyncTask.getRootCommandId(), > please extract to method this trinary if - it appears more than once. The second call needs to getCommandId, looks like a Bug in the code Line 39: VdcActionType.Unknown, Line 40: new VdcActionParametersBase()), Line 41: getCommandEntity(coco, Line 42: asyncTask == null ? Guid.newGuid() : asyncTask.getRootCommandId(), Line 51: private static CommandEntity getCommandEntity(CommandCoordinator coco, Line 52: Guid cmdId, Line 53: VdcActionType actionType, Line 54: VdcActionParametersBase paramaters) { Line 55: CommandEntity cmdEntity = Guid.isNullOrEmpty(cmdId) ? null : coco.getCommandEntity(cmdId); > Please move the Guid.isNullOrEmpty into coco.getCommandEntity. Done Line 56: if (cmdEntity == null) { Line 57: cmdEntity = new CommandEntity(); Line 58: cmdEntity.setId(cmdId); Line 59: cmdEntity.setCommandType(actionType); Line 56: if (cmdEntity == null) { Line 57: cmdEntity = new CommandEntity(); Line 58: cmdEntity.setId(cmdId); Line 59: cmdEntity.setCommandType(actionType); Line 60: cmdEntity.setCommandParameters(paramaters); > Do you need to set the new create command in coco? Done Line 61: } Line 62: return cmdEntity; Line 63: } Line 64: http://gerrit.ovirt.org/#/c/31737/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CoCoAsyncTaskHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CoCoAsyncTaskHelper.java: Line 154: return asyncTasks; Line 155: } Line 156: Line 157: private CommandEntity getCommandEntity(Guid cmdId) { Line 158: CommandEntity cmdEntity = Guid.isNullOrEmpty(cmdId) ? null : coco.getCommandEntity(cmdId); > so you do have the check for null or empty guid, see the comment in the pre Done Line 159: if (cmdEntity == null) { Line 160: cmdEntity = new CommandEntity(); Line 161: cmdEntity.setId(cmdId); Line 162: cmdEntity.setCommandType(VdcActionType.Unknown); Line 175: @Override Line 176: public Void runInTransaction() { Line 177: DbFacade.getInstance().getAsyncTaskDao().save(asyncTask); Line 178: if (!Guid.isNullOrEmpty(asyncTask.getRootCommandId())) { Line 179: coco.persistCommand(asyncTask.getParentCmdEntity()); > Hi, Done Line 180: } Line 181: if (!Guid.isNullOrEmpty(asyncTask.getCommandId())) { Line 182: coco.persistCommand(asyncTask.getChildCmdEntity()); Line 183: } Line 188: Line 189: public AsyncTasks getAsyncTaskFromDb(Guid asyncTaskId) { Line 190: AsyncTasks asyncTask = DbFacade.getInstance().getAsyncTaskDao().get(asyncTaskId); Line 191: if (asyncTask != null) { Line 192: asyncTask.setParentCmdEntity(getCommandEntity(asyncTask.getRootCommandId())); > See previous question Done Line 193: asyncTask.setChildCmdEntity(getCommandEntity(asyncTask.getCommandId())); Line 194: } Line 195: return asyncTask; Line 196: } Line 205: if (asyncTask != null && !Guid.isNullOrEmpty(asyncTask.getCommandId())) { Line 206: CommandEntity cmdEntity = coco.getCommandEntity(asyncTask.getCommandId()); Line 207: if (cmdEntity != null && !cmdEntity.isCallBackEnabled()) { Line 208: coco.removeCommand(asyncTask.getCommandId()); Line 209: if (!Guid.isNullOrEmpty(asyncTask.getRootCommandId()) && > ok, so now i am confused, u do have getRootCommandId? so is the check in li this check is not required, changed Line 210: !coco.hasCommandEntitiesWithRootCommandId(asyncTask.getRootCommandId())) { Line 211: coco.removeCommand(asyncTask.getRootCommandId()); Line 212: } Line 213: } Line 253: Line 254: @Override Line 255: public Void runInTransaction() { Line 256: if (!Guid.isNullOrEmpty(asyncTask.getRootCommandId())) { Line 257: coco.persistCommand(asyncTask.getParentCmdEntity()); > similar question as before. Done Line 258: } Line 259: if (!Guid.isNullOrEmpty(asyncTask.getCommandId())) { Line 260: coco.persistCommand(asyncTask.getChildCmdEntity()); Line 261: } Line 330: VdcActionParametersBase parameters) { Line 331: CommandEntity cmdEntity = Guid.isNullOrEmpty(cmdId) ? null : coco.getCommandEntity(cmdId); Line 332: if (cmdEntity == null) { Line 333: cmdEntity = new CommandEntity(); Line 334: cmdEntity.setId(cmdId); > should we place the created command entity in coco? Done Line 335: cmdEntity.setCommandType(actionType); Line 336: cmdEntity.setCommandParameters(parameters); Line 337: if (!Guid.isNullOrEmpty(cmdId)) { Line 338: cmdEntity.setCommandStatus(CommandStatus.ACTIVE); http://gerrit.ovirt.org/#/c/31737/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/AsyncTasks.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/AsyncTasks.java: Line 24: private Date startTime; Line 25: private CommandEntity parentCmdEntity; Line 26: private CommandEntity childCmdEntity; Line 27: Line 28: public AsyncTasks() { > Please please rename to AsyncTask :) Done Line 29: result = AsyncTaskResultEnum.success; Line 30: status = AsyncTaskStatusEnum.unknown; Line 31: vdsmTaskId = Guid.Empty; Line 32: commandId = Guid.Empty; Line 48: this.vdsmTaskId = vdsmTaskId; Line 49: this.stepId = stepId; Line 50: this.startTime = new Date(); Line 51: this.commandId = childCmdEntity.getId(); Line 52: this.rootCommandId = parentCmdEntity.getId(); > confusing... parent or root? changed to root Line 53: this.storagePoolId = storagePoolId; Line 54: this.taskId = Guid.newGuid(); Line 55: this.taskType = taskType; Line 56: this.parentCmdEntity = parentCmdEntity; Line 56: this.parentCmdEntity = parentCmdEntity; Line 57: this.childCmdEntity = childCmdEntity; Line 58: } Line 59: Line 60: public VdcActionType getaction_type() { > any chance of getting rid of non java conventions in method names? :) Done Line 61: return parentCmdEntity.getCommandType(); Line 62: } Line 63: Line 64: public void setaction_type(VdcActionType value) { -- To view, visit http://gerrit.ovirt.org/31737 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ec5c072c6f3153c9396b0ce0be8e7c59cbefea1 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@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
