Gilad Chaplik has posted comments on this change. Change subject: db: aggregate qos and storage qos impl ......................................................................
Patch Set 6: (6 comments) http://gerrit.ovirt.org/#/c/27094/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/BaseQosDaoFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/BaseQosDaoFacadeImpl.java: Line 69: Line 70: @Override Line 71: protected MapSqlParameterSource createFullParametersMapper(T obj) { Line 72: MapSqlParameterSource map = getCustomMapSqlParameterSource() Line 73: .addValue("id", obj.getId()) > reuse createIdParameterMapper(obj.getId()) which you defined next. Done Line 74: .addValue("qos_type", getQosType()) Line 75: .addValue("name", obj.getName()) Line 76: .addValue("description", obj.getDescription()) Line 77: .addValue("storage_pool_id", obj.getStoragePoolId()); Line 85: return getCustomMapSqlParameterSource() Line 86: .addValue("id", guid); Line 87: } Line 88: Line 89: protected Integer getIntegerOrNull(ResultSet rs, String columnName) throws SQLException { > why this is required ? the control over the db population and querying is y I guess it was copied from code in vnics. Done. Line 90: int i = rs.getInt(columnName); Line 91: return rs.wasNull() ? null : i; Line 92: } http://gerrit.ovirt.org/#/c/27094/6/packaging/dbscripts/qos_sp.sql File packaging/dbscripts/qos_sp.sql: Line 65: BEGIN Line 66: RETURN QUERY SELECT * Line 67: FROM qos Line 68: WHERE storage_pool_id = v_storage_pool_id Line 69: AND (v_qos_type IS NULL OR qos_type = v_qos_type); > why v_qos_type should be null ? when you pass null, you get all. you don't like it ahh? :-) Line 70: END; $procedure$ Line 71: LANGUAGE plpgsql; http://gerrit.ovirt.org/#/c/27094/6/packaging/dbscripts/upgrade/03_05_0330_qos_and_storage_impl.sql File packaging/dbscripts/upgrade/03_05_0330_qos_and_storage_impl.sql: Line 7: id uuid NOT NULL, Line 8: qos_type SMALLINT NOT NULL, Line 9: name VARCHAR(50) NOT NULL, Line 10: description TEXT, Line 11: storage_pool_id uuid NOT NULL, > please rename any references to storage_pool to data_center. Done Line 12: max_throughput INTEGER, Line 13: max_read_throughput INTEGER, Line 14: max_write_throughput INTEGER, Line 15: max_iops INTEGER, Line 21: ) WITH OIDS; Line 22: Line 23: ALTER TABLE qos ADD CONSTRAINT fk_qos_storage_pool FOREIGN KEY (storage_pool_id) Line 24: REFERENCES storage_pool (id) MATCH SIMPLE Line 25: ON UPDATE NO ACTION ON DELETE CASCADE; > cascade deletion is tricky - you should handle compensation in RemoveStorag I don't get it. remove DC in a transaction. so we rollback everything manually?? that's a big hole. Line 26: Line 27: -- add index on storage_pool_id Line 28: CREATE INDEX IDX_qos_storage_pool_id ON qos (storage_pool_id); Line 29: Line 27: -- add index on storage_pool_id Line 28: CREATE INDEX IDX_qos_storage_pool_id ON qos (storage_pool_id); Line 29: Line 30: -- add index on qos_type Line 31: CREATE INDEX IDX_qos_type ON qos (qos_type); > i don't think this is a valid index. You only expose s/p for selecting by d it is in the UI: DataCenters main tab/ QoS sub tab/ get QoS by Type. -- To view, visit http://gerrit.ovirt.org/27094 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c0051d63b Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kobi Ianko <k...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@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
