----- Original Message ----- > > > ----- Original Message ----- > > From: "Moti Asayag" <[email protected]> > > To: [email protected] > > 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 > > [email protected] > > http://lists.ovirt.org/mailman/listinfo/engine-devel > > > _______________________________________________ > Engine-devel mailing list > [email protected] > http://lists.ovirt.org/mailman/listinfo/engine-devel > _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
