Re: [Engine-devel] Introducing virt / gluster flags at cluster level

----- Original Message ----- > > > ----- Original Message ----- >> From: "Moti Asayag"<masayag(a)redhat.com> >> To: engine-devel(a)ovirt.org >> Sent: Sunday, March 4, 2012 12:20:43 PM >> Subject: Re: [Engine-devel] Introducing virt / gluster flags at >> cluster level >> >> On 03/01/2012 12:54 PM, Shireesh Anjal wrote: >>> Hi, >>> >>> In order to identify whether a cluster exposes Gluster / >>> Virtualization >>> capabilities, we plan to introduce two boolean columns - >>> virt_service >>> and gluster_service in the vds_groups table. As per immediate >>> plans, it >>> is intended to support only one service per cluster, meaning only >>> one of >>> these two values can be true. >> >> Couldn't there be additional future services in the future ? In >> that >> case perhaps worth considering enum for services, to be stored in a >> single service column, its values are: virt, gluster,...) instead >> of >> extending the vds_group table consistently when introducing new >> services >> (under the assumption no mix of services is allowed). > > +1 for an Enum insteand of boolean columns. we have too much of that > already and eventually we see long records and > routine refactoring to our DAL. > Also to make mixed configuration we can embrace bit fields which > interacts very nice with Enums e.g a 5 value of is a cluster with > VIRT(1) and FUTURE(4) capabilities I agree about the columns being inflexible but personally I don't like enums. What if we need to support finer grained services? e.g. different topologies (active/passive, active/active etc) or other types of intricate relationships? Not to mention that looking at the raw data you don't undetstand what it means without holding a dictionary. And it's annoying to support BC of the numbers once things change.

> >> >>> >>> We plan to make following changes as part of the patch: >>> >>> 1) Introduce an upgrade script for adding following columns to >>> the >>> table >>> "vds_groups" >>> - virt_service - boolean - NOT_NULL - default value true >>> - gluster_service - boolean - NOT_NULL - default value false >>> >>> => What is the naming convention for naming the upgrade script? >>> Is >>> it >>> <major-version>_<minor-version>_<running-sequence-with-step-10>_script_description.sql >>> ? By that logic, the name of our script will be something like >>> 03_01_0560_add_service_columns_to_vds_groups.sql >> >> Correct. Make sure to verify script sequence uniqueness (under >> ovirt-engine/backend/manager/dbscripts/upgrade) before pushing, >> since >> there is no enforcement for that. >> >>> >>> 2) Modify vds_groups_sp.sql to introduce these arguments / >>> variables in >>> save and update methods >>> >>> 3) Modify the DAO interface and implementation to introduce these >>> arguments / variables in the save/update methods and >>> VdsGroupRawMapper. >>> >>> 4) Modify the DAO JUnit test class to take care of these new >>> fields, if >>> required. >> >> As part of DAO unit-test you should add the new columns and >> possible >> data to >> ovirt-engine/backend/manager/modules/dal/src/test/resources/fixtures.xml >> >>> >>> 5) Modify class VDSGroup to introduce the two new boolean >>> variables >>> virtService and glusterService and modify the methods equals and >>> hashCode to use these. >>> >>> => One question here. Most of the attributes of class VDSGroup >>> have >>> annotations like @XmlElement and @Column. I think these are >>> related >>> to >>> jaxb and JPA. Are these annotations really required? If yes, how >>> are >>> they useful? >>> >> >> @XmlElement annotations could be deleted from that file as a prior >> patch >> to the patch-set. >> >> JPA annotations could be ignored as not fully supported nor >> maintained. >> >>> 6) REST API : Default populate these variables (virtService = >>> true, >>> glusterService = false) before invoking the BLL command >>> (AddVdsGroupCommand / UpdateVdsGroupCommand). This makes sure >>> that >>> the >>> existing API won't break. No change in api.xsd for now. >>> >>> 7) webadmin create cluster code: Default populate the new >>> variables, >>> same as above. No change in UI screen for now. >>> >>> => Alternatively, the variable declaration on VDSGroup itself can >>> initialize these variables with default values, so that changes >>> to >>> UI / >>> REST API code (points 6 and 7) may not be required immediately. >>> What do >>> you suggest? >>> >>> Inputs / comments / suggestions welcome. >>> >> >> _______________________________________________ >> Engine-devel mailing list >> Engine-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/engine-devel >> > _______________________________________________ > Engine-devel mailing list > Engine-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ Engine-devel mailing list Engine-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel