Moti Asayag has posted comments on this change. Change subject: core: Support transactions for Quartz jobs ......................................................................
Patch Set 7: (4 comments) http://gerrit.ovirt.org/#/c/36370/7//COMMIT_MSG Commit Message: Line 9: Add annotation that allows Quartz methods to automatically run in Line 10: a transaction Line 11: Line 12: Change-Id: I7eb67e6aeef9080b5647cb84b1cf701c720ce29d Line 13: Bug-Url: https://bugzilla.redhat.com/?????? Is there a bug what should be associated with this patch ? http://gerrit.ovirt.org/#/c/36370/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/JobRepositoryCleanupManager.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/job/JobRepositoryCleanupManager.java: Line 50: * <li>The successful jobs will be deleted after {@code ConfigValues.SucceededJobCleanupTimeInMinutes}.</li> Line 51: * <li>The failed jobs will be deleted after {@code ConfigValues.FailedJobCleanupTimeInMinutes}.</li> Line 52: * <ul> Line 53: */ Line 54: @OnTimerMethodAnnotation(value = "completed_jobs_cleanup", transactional = true) why this is required ? Line 55: public void cleanCompletedJob() { Line 56: Line 57: Date succeededJobsDeleteTime = Line 58: new Date(System.currentTimeMillis() - TimeUnit.MILLISECONDS.convert(succeededJobTime, TimeUnit.MINUTES)); http://gerrit.ovirt.org/#/c/36370/7/backend/manager/modules/scheduler/src/main/java/org/ovirt/engine/core/utils/timer/JobWrapper.java File backend/manager/modules/scheduler/src/main/java/org/ovirt/engine/core/utils/timer/JobWrapper.java: Line 42: final Object instance = paramsMap.get(SchedulerUtilQuartzImpl.RUNNABLE_INSTANCE); Line 43: final Object[] methodParams = (Object[]) paramsMap.get(SchedulerUtilQuartzImpl.RUN_METHOD_PARAM); Line 44: String methodKey = getMethodKey(instance.getClass().getName(), methodName); Line 45: final Method methodToRun; Line 46: if (!cachedMethods.containsKey(methodKey)) { can't this synchronized block be replaced by ConcurrentHashMap.putIfAbsent (methodKey, methodToRun) ? Line 47: synchronized (cachedMethods) { Line 48: if (cachedMethods.containsKey(methodKey)) { Line 49: methodToRun = cachedMethods.get(methodKey); Line 50: } else { Line 67: @Override Line 68: public Exception runInTransaction() { Line 69: try { Line 70: methodToRun.invoke(instance, methodParams); Line 71: } catch (Exception e) { why can't the return type be void ? If an exception is thrown by " methodToRun.invoke(instance, methodParams);" it will be propagated. Line 72: return e; Line 73: } Line 74: return null; Line 75: } -- To view, visit http://gerrit.ovirt.org/36370 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7eb67e6aeef9080b5647cb84b1cf701c720ce29d Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@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
