Eli Mesika has posted comments on this change. Change subject: db: Numa support database implementation ......................................................................
Patch Set 23: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/26996/23/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 44: -- Create partial index for vds cpu statistics Line 45: CREATE INDEX IDX_vds_cpu_statistics_vds_id ON vds_cpu_statistics(vds_id); Line 46: Line 47: -- Numa nodes internal connection table Line 48: CREATE TABLE vm_vds_numa_node_map IMHO this should be implemented as separate tables one for vms and one for vds There is no point in merging those tables to one I can see some troubles in doing that in the same table, for example potential deadlocks... Line 49: ( Line 50: id UUID NOT NULL, Line 51: vm_numa_node_id UUID NOT NULL, Line 52: vds_numa_node_id UUID, Line 47: -- Numa nodes internal connection table Line 48: CREATE TABLE vm_vds_numa_node_map Line 49: ( Line 50: id UUID NOT NULL, Line 51: vm_numa_node_id UUID NOT NULL, Also, this got a id and additional vm_numa_node_id and vds_numa_node_id How comes that the vm_numa_node_id is not null while the vds_numa_node_id can be null, AFAIK in each record you have only one of those IDs This demonstrate also the problem in forcing this to be implemented in one table You should have vm_numa_node_map with vm_numa_node_id as PK and vds_numa_node_map with vds_numa_node_id as a PK Line 52: vds_numa_node_id UUID, Line 53: vds_numa_node_index SMALLINT, Line 54: is_pinned BOOLEAN DEFAULT false NOT NULL, Line 55: CONSTRAINT pk_vm_vds_numa_node_map PRIMARY KEY(id), http://gerrit.ovirt.org/#/c/26996/23/packaging/dbscripts/upgrade/post_upgrade/0030_add_numa_col_in_white_list_table.sql File packaging/dbscripts/upgrade/post_upgrade/0030_add_numa_col_in_white_list_table.sql: Line 1: -- Add new columns for numa feature Please see the comment in 0010_add_object_column_white_list_table.sql at the end of the file , you should add those modification in 0010_add_object_column_white_list_table.sql No need for the SP Line 2: Create or replace FUNCTION __temp_add_numa_columns() returns void Line 3: AS $procedure$ Line 4: BEGIN Line 5: -- Add rows in table object_column_white_list -- 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: 23 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
