Gilad Chaplik has posted comments on this change. Change subject: db: aggregate qos and storage qos impl ......................................................................
Patch Set 7: (6 comments) http://gerrit.ovirt.org/#/c/27094/7/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java: Line 188: put(VmInit.class, VmInitDAO.class); Line 189: put(CpuStatistics.class, VdsCpuStatisticsDAO.class); Line 190: put(VdsNumaNode.class, VdsNumaNodeDAO.class); Line 191: put(VmNumaNode.class, VmNumaNodeDAO.class); Line 192: put(StorageQos.class, StorageQosDao.class); > this map is used for compensation, as currently we don't use it for qos , i http://gerrit.ovirt.org/#/c/27094/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacade.java have you talk it over and reached resolutions? Line 193: } Line 194: }; Line 195: Line 196: private JdbcTemplate jdbcTemplate; http://gerrit.ovirt.org/#/c/27094/7/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/BaseDAODbFacade.java: Line 129: * @param columnName Line 130: * @return a Integer or null Line 131: * @throws SQLException Line 132: */ Line 133: protected final static Integer getInteger(ResultSet resultSet, String columnName) throws SQLException { > I'd change this method implementation to: Done. Line 134: if (resultSet.getInt(columnName) == 0 && resultSet.wasNull()) { Line 135: return null; Line 136: } else { Line 137: return resultSet.getInt(columnName); http://gerrit.ovirt.org/#/c/27094/7/packaging/dbscripts/upgrade/03_05_0330_qos_and_storage_impl.sql File packaging/dbscripts/upgrade/03_05_0330_qos_and_storage_impl.sql: Line 16: max_read_iops INTEGER, Line 17: max_write_iops INTEGER, Line 18: _create_date TIMESTAMP WITH TIME ZONE default LOCALTIMESTAMP, Line 19: _update_date TIMESTAMP WITH TIME ZONE default NULL, Line 20: CONSTRAINT PK_qos_id PRIMARY KEY (id) > 1. you can declare the colum as primary key 2. we decided to go on a sparse matrix approach. Allon and Moti aware of it and ACKed. 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 20: CONSTRAINT PK_qos_id PRIMARY KEY (id) 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 > MATCH SIMPLE can be removed as it's the default Done Line 25: ON UPDATE NO ACTION ON DELETE CASCADE; 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 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; > no need to specify the ON UPDATE part. I want to explicitly specify that. don't want any future questions that it was maybe forgotten. 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); > wasn't it agreed to remove the indexes or to create one for both columns? Done -- 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: 7 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: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@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
