Eli Mesika has posted comments on this change.

Change subject: gluster: Tables, SPs and DAOs for gluster services
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/dbscripts/upgrade/03_03_0120_add_gluster_services_tables.sql
Line 3: BEGIN
Line 4:     -- Service Types
Line 5:     CREATE TABLE gluster_service_types
Line 6:     (
Line 7:         service_type VARCHAR(100) NOT NULL,
What is the potential size of those tables ?
If can be large , I prefer an additional  service_type_id that be used for 
comparing 
This can be a sequence or UUID ... 
If its a small table even in large installations , I don't mind leaving as is
Line 8:         CONSTRAINT pk_gluster_service_types PRIMARY KEY(service_type)
Line 9:     ) WITH OIDS;
Line 10: 
Line 11:     -- Services ( There can be multiple services under a given service 
type )


Line 22:     CREATE TABLE gluster_cluster_services
Line 23:     (
Line 24:         cluster_id UUID NOT NULL references vds_groups(vds_group_id) 
ON DELETE CASCADE,
Line 25:         service_type VARCHAR(100) NOT NULL references 
gluster_service_types(service_type) ON DELETE CASCADE,
Line 26:         status VARCHAR(100) NOT NULL,
why varchar(100) for a status column ?
Line 27:         _create_date TIMESTAMP WITH TIME ZONE NOT NULL default 
LOCALTIMESTAMP,
Line 28:         _update_date TIMESTAMP WITH TIME ZONE,
Line 29:         CONSTRAINT pk_gluster_cluster_services PRIMARY KEY(cluster_id, 
service_type)
Line 30:     ) WITH OIDS;


Line 38:         id UUID NOT NULL,
Line 39:         server_id UUID NOT NULL references vds_static(vds_id) ON 
DELETE CASCADE,
Line 40:         service_id UUID NOT NULL references gluster_services(id) ON 
DELETE CASCADE,
Line 41:         pid INTEGER,
Line 42:         status VARCHAR(100) NOT NULL,
again , please align size to real needs
Line 43:         message VARCHAR(1000),
Line 44:         _create_date TIMESTAMP WITH TIME ZONE NOT NULL default 
LOCALTIMESTAMP,
Line 45:         _update_date TIMESTAMP WITH TIME ZONE,
Line 46:         CONSTRAINT pk_gluster_server_services PRIMARY KEY(id),


--
To view, visit http://gerrit.ovirt.org/14344
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ded8d39203009bc66974e136c41b5de180ffbff
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to