Liran Zelkha has posted comments on this change. Change subject: core: Move Job, Step to JPA ......................................................................
Patch Set 51: (5 comments) https://gerrit.ovirt.org/#/c/34553/51/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/job/JobSubjectEntity.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/job/JobSubjectEntity.java: Line 47: @Type(type = "org.ovirt.engine.core.dao.jpa.GuidUserType") Line 48: private Guid jobId; Line 49: Line 50: public JobSubjectEntity() { Line 51: super(); > please remove empty c'tor Done Line 52: } Line 53: Line 54: public JobSubjectEntity(Guid jobId, Guid entityId, VdcObjectType entityType) { Line 55: super(); Line 51: super(); Line 52: } Line 53: Line 54: public JobSubjectEntity(Guid jobId, Guid entityId, VdcObjectType entityType) { Line 55: super(); > please remove empty c'tor Done Line 56: this.entityId = entityId; Line 57: this.entityType = entityType; Line 58: this.jobId = jobId; Line 59: } https://gerrit.ovirt.org/#/c/34553/51/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/job/JobSubjectEntityId.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/job/JobSubjectEntityId.java: Line 33: Line 34: @Override Line 35: public boolean equals(Object obj) { Line 36: if (this == obj) Line 37: return true; > repeated twice Done Line 38: if (this == obj) { Line 39: return true; Line 40: } Line 41: if (!(obj instanceof JobSubjectEntityId)) { https://gerrit.ovirt.org/#/c/34553/51/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/JobDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/JobDaoDbFacadeImpl.java: Line 42: allJobs.addAll(multipleResults(entityManager.createNamedQuery("Job.getJobsByOffsetAndPageSizeNotInStatus", Line 43: Job.class) Line 44: .setParameter("status", EnumSet.of(JobExecutionStatus.STARTED, Line 45: JobExecutionStatus.UNKNOWN)))); Line 46: int endIndex = Math.min(offset + pageSize, allJobs.size()); > how would this code behave ? those entities are cache-able, but if we happe Since we need to have 2 queries (JPA doesn't support UNION), the limit we can do is problematic. We would in fact load all records (actually, all records from the table), and then return only some of them. Line 47: return allJobs.subList(offset, endIndex); Line 48: } Line 49: Line 50: @Override https://gerrit.ovirt.org/#/c/34553/51/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StepDaoDbFacadeImpl.java: Line 44: } Line 45: Line 46: @Override Line 47: public void updateJobStepsCompleted(Guid jobId, JobExecutionStatus status, Date endTime) { Line 48: if (status != JobExecutionStatus.STARTED) { > this introduces business logic into the data layer. It was in the stored procedure before... Line 49: updateQuery(entityManager.createNamedQuery("Step.updateJobStepsCompleted") Line 50: .setParameter("jobId", jobId) Line 51: .setParameter("status", status) Line 52: .setParameter("endTime", endTime) -- To view, visit https://gerrit.ovirt.org/34553 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifcfda7b055d37c92c1346b100101c27d594d21fb Gerrit-PatchSet: 51 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liran Zelkha <lzel...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
