Sahina Bose has posted comments on this change. Change subject: gluster: BLL queries for volume snapshots list and count ......................................................................
Patch Set 12: (8 comments) http://gerrit.ovirt.org/#/c/35674/12/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterVolumeDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterVolumeDaoDbFacadeImpl.java: Line 338: } Line 339: volume.setBricks(bricks); Line 340: int snapshotsCount = Line 341: dbFacade.getGlusterVolumeDao().getSnapshotCount(volume.getId()); Line 342: volume.setNoOfSnapshots(snapshotsCount); Can this be not returned in the view - gluster_volumes_view? Is there a need to call a stored procedure to get this detail? Line 343: GlusterVolumeSnapshotConfig config = Line 344: dbFacade.getGlusterVolumeSnapshotConfigDao() Line 345: .getConfigByVolumeIdAndName(volume.getClusterId(), Line 346: volume.getId(), Line 343: GlusterVolumeSnapshotConfig config = Line 344: dbFacade.getGlusterVolumeSnapshotConfigDao() Line 345: .getConfigByVolumeIdAndName(volume.getClusterId(), Line 346: volume.getId(), Line 347: "snap-max-hard-limit"); Consider externalising string Line 348: if (config == null || config.getParamValue() == null || config.getParamValue().equals("")) { Line 349: config = Line 350: dbFacade.getGlusterVolumeSnapshotConfigDao() Line 351: .getConfigByClusterIdAndName(volume.getClusterId(), http://gerrit.ovirt.org/#/c/35674/12/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterVolumeSnapshotDao.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/GlusterVolumeSnapshotDao.java: Line 22: public List<GlusterVolumeSnapshotEntity> getAllWithQuery(String query); Line 23: Line 24: public void remove(Guid id); Line 25: Line 26: public void removeAll(Guid volumeId, Collection<Guid> ids); why is volume id required? Collection of ids - is a list of snapshot ids? Line 27: Line 28: public void removeAllByVolumeId(Guid volumeId); Line 29: Line 30: public void removeByName(Guid volumeId, String snapshotName); http://gerrit.ovirt.org/#/c/35674/12/packaging/dbscripts/gluster_volume_snapshot_sp.sql File packaging/dbscripts/gluster_volume_snapshot_sp.sql: Line 64: AS $procedure$ Line 65: DECLARE Line 66: ref_volume_id UUID; Line 67: BEGIN Line 68: ref_volume_id := volume_id FROM gluster_volume_snapshots WHERE snapshot_id = v_snapshot_id; SELECT volume_id INTO ref_volume_id syntax here? Line 69: Line 70: DELETE FROM gluster_volume_snapshots Line 71: WHERE snapshot_id = v_snapshot_id; Line 72: Line 83: DECLARE Line 84: del_count INTEGER; Line 85: BEGIN Line 86: del_count := count(*) FROM gluster_volume_snapshots WHERE volume_id = v_volume_id; Line 87: same comment as before Line 88: DELETE FROM gluster_volume_snapshots Line 89: WHERE volume_id = v_volume_id; Line 90: Line 91: UPDATE gluster_volumes Line 86: del_count := count(*) FROM gluster_volume_snapshots WHERE volume_id = v_volume_id; Line 87: Line 88: DELETE FROM gluster_volume_snapshots Line 89: WHERE volume_id = v_volume_id; Line 90: TWS Line 91: UPDATE gluster_volumes Line 92: SET snapshot_count = snapshot_count - del_count Line 93: WHERE id = v_volume_id; Line 94: END; $procedure$ Line 99: v_snapshot_name VARCHAR(1000)) Line 100: RETURNS VOID Line 101: AS $procedure$ Line 102: DECLARE Line 103: ref_volume_id UUID; Not required? Line 104: BEGIN Line 105: DELETE FROM gluster_volume_snapshots Line 106: WHERE volume_id = v_volume_id Line 107: AND snapshot_name = v_snapshot_name; Line 119: AS $procedure$ Line 120: DECLARE Line 121: del_count INTEGER; Line 122: BEGIN Line 123: del_count := count(*) FROM fnSplitterUuid(v_snapshot_ids); No check that the snapshot ids belong to v_volume_id. Instead of passing volume_id as a parameter, can you not get the volumeid and count from the list of snapshot_ids and update count? select volume_id, count(volume_id) from gluster_volume_snapshots 125 WHERE snapshot_id in (select * from fnSplitterUuid(v_snapshot_ids)) group by volume_id; You can get the result of above into REFCURSOR Line 124: Line 125: DELETE FROM gluster_volume_snapshots Line 126: WHERE snapshot_id in (select * from fnSplitterUuid(v_snapshot_ids)); Line 127: -- To view, visit http://gerrit.ovirt.org/35674 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00cd2a52c9bd10946b8702d833d6f8f0ebb3f848 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@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
