Xiaolei Shi has posted comments on this change. Change subject: db: Numa support database implementation ......................................................................
Patch Set 21: (13 comments) http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsCpuStatisticsDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsCpuStatisticsDAO.java: Line 47: * Line 48: * @param vdsId Line 49: * the vds id Line 50: */ Line 51: void removeAllCpuStatisticsByVdsId(Guid vdsId); > that should be handled by delete cascade, nop? If host is removed, then cpu statistics will be deleted cascade. Adding CPUs to host after reboot, will check the cpu core id match. If reported cpu core ids matches the already saved, then do mass update; if not matches, will call this method to remove all and then insert. Line 52: http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsCpuStatisticsDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsCpuStatisticsDAODbFacadeImpl.java: Line 46: MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() Line 47: .addValue("vds_id", vdsId); Line 48: Line 49: getCallsHandler().executeModification("DeleteVdsCpuStatisticsByVdsId", parameterSource); Line 50: } > see last comment Answered in that comment. Line 51: Line 52: private MapSqlParameterSource createCpuStatisticsParametersMapper(CpuStatistics stat) { Line 53: return getCustomMapSqlParameterSource() Line 54: .addValue("cpu_core_id", stat.getCpuId()) http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java: Line 260: .addValue("supported_emulated_machines", vds.getSupportedEmulatedMachines()) Line 261: .addValue("kdump_status", vds.getKdumpStatus().getAsNumber()) Line 262: .addValue("auto_numa_balancing", vds.getAutoNumaBalancing().getValue()) Line 263: .addValue("vds_numa_node_count", Line 264: vds.getNumaNodeList() == null ? 0 : vds.getNumaNodeList().size()) > remove. Done Line 265: .addValue("is_numa_supported", vds.isNumaSupport()); Line 266: Line 267: return parameterSource; Line 268: } http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsNumaNodeDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsNumaNodeDAO.java: Line 9: * <code>VdsNumaNodeDAO</code> defines a type for performing CRUD operations on instances of {@link VdsNumaNode}. Line 10: * Line 11: * Line 12: */ Line 13: public interface VdsNumaNodeDAO extends DAO { > this iface accommodates iface VmNumaNodeDao as well. Done Line 14: Line 15: /** Line 16: * Get all numa nodes of a vds by vds id Line 17: * @param vdsId Line 28: * the vds numa nodes to be saved Line 29: * @param vdsId Line 30: * the vds id that the numa nodes belong to Line 31: */ Line 32: void massSaveVdsNumaNode(List<VdsNumaNode> vdsNumaNodes, Guid vdsId); > plural Done Line 33: Line 34: /** Line 35: * Update the statistics data of the given list of vds numa nodes using a more efficient method Line 36: * to update all of them at once, rather than each at a time. Line 37: * Line 38: * @param nodes Line 39: * the vds numa nodes to be updated Line 40: */ Line 41: void massUpdateVdsNumaNodeStatistics(List<VdsNumaNode> vdsNumaNodes); > plural Done Line 42: Line 43: /** Line 44: * Update non-statistics data of the given list of vds numa nodes using a more efficient method Line 45: * to update all of them at once, rather than each at a time. Line 46: * Line 47: * @param nodes Line 48: * the vds numa nodes to be updated Line 49: */ Line 50: void massUpdateVdsNumaNode(List<VdsNumaNode> vdsNumaNodes); > plural Done Line 51: Line 52: /** Line 53: * Remove all the vds numa node of a given vds Line 54: * Line 63: * Line 64: * @param vdsNumaNodeIds Line 65: * the vds numa node ids to be removed Line 66: */ Line 67: void massRemoveVdsNumaNodeByVdsNumaNodeId(List<Guid> vdsNumaNodeIds); > plural Done Line 68: http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDAO.java: Line 10: * <code>VmNumaNodeDAO</code> defines a type for performing CRUD operations on instances of {@link VmNumaNode}. Line 11: * Line 12: * Line 13: */ Line 14: public interface VmNumaNodeDAO extends DAO { > should extend vdsNumaNodeDao. see comment there. Done Line 15: Line 16: /** Line 17: * Get all numa nodes of a vm by vm id Line 18: * @param vmId http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDAODbFacadeImpl.java: Line 14: import org.ovirt.engine.core.compat.Guid; Line 15: import org.springframework.jdbc.core.RowMapper; Line 16: import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; Line 17: Line 18: public class VmNumaNodeDAODbFacadeImpl extends BaseDAODbFacade implements VmNumaNodeDAO { > a lot of code duplication with vdsDaoImpl class, please handle. Done Line 19: Line 20: @Override Line 21: public List<VmNumaNode> getAllVmNumaNodeByVmId(Guid vmId) { Line 22: MapSqlParameterSource parameterSource = getCustomMapSqlParameterSource() http://gerrit.ovirt.org/#/c/26996/21/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmStaticDAODbFacadeImpl.java: Line 103: .addValue("serial_number_policy", vm.getSerialNumberPolicy() == null ? null : vm.getSerialNumberPolicy().getValue()) Line 104: .addValue("custom_serial_number", vm.getCustomSerialNumber()) Line 105: .addValue("is_boot_menu_enabled", vm.isBootMenuEnabled()) Line 106: .addValue("numatune_mode", vm.getNumaTuneMode().getValue()) Line 107: .addValue("vm_numa_node_count", vm.getvNumaNodeList() == null ? 0 : vm.getvNumaNodeList().size()); > -1, who said vNumaNodeList() is filled? ok. I will update this count in our dao when save or delete vNumaNode. Is that ok? Line 108: } Line 109: Line 110: @Override Line 111: public void remove(Guid id) { http://gerrit.ovirt.org/#/c/26996/21/packaging/dbscripts/numa_sp.sql File packaging/dbscripts/numa_sp.sql: Line 4: -- [numa_node] Table Line 5: -- Line 6: Line 7: Line 8: Create or replace FUNCTION InsertNumaNode(v_numa_node_id UUID, > insert vm numa nude should include mapping, what do you say? It affects two tables, that will be done in dao with transaction. Line 9: v_vds_id UUID, Line 10: v_vm_id UUID, Line 11: v_numa_node_index SMALLINT, Line 12: v_mem_total BIGINT, http://gerrit.ovirt.org/#/c/26996/21/packaging/dbscripts/upgrade/03_05_0330_add_numa_tables_and_columns.sql File packaging/dbscripts/upgrade/03_05_0330_add_numa_tables_and_columns.sql: Line 90: PERFORM fn_db_add_column('vm_static', 'vm_numa_node_count', 'smallint'); Line 91: Line 92: -- Add columns in table vds_dynamic Line 93: PERFORM fn_db_add_column('vds_dynamic', 'auto_numa_balancing', 'smallint'); Line 94: PERFORM fn_db_add_column('vds_dynamic', 'vds_numa_node_count', 'smallint'); > this field can be removed. Need this field to do the query in future. We will provide the query that find host with specified numa node number. If no this field, we will do a table join and even count in db, that will be low performance. I will add a trigger to update the two count values. Is it ok? Line 95: PERFORM fn_db_add_column('vds_dynamic', 'is_numa_supported', 'boolean'); Line 96: Line 97: END; $procedure$ Line 98: LANGUAGE plpgsql; -- To view, visit http://gerrit.ovirt.org/26996 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2507c084aa214bcfb65e860b11ed7dcf02af50cc Gerrit-PatchSet: 21 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Xiaolei Shi <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Jason Liao <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Xiaolei Shi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
