----- Original Message ----- > From: "Itamar Heim" <[email protected]> > To: "Ayal Baron" <[email protected]> > Cc: [email protected] > Sent: Sunday, March 4, 2012 8:08:52 PM > Subject: Re: [Engine-devel] Introducing virt / gluster flags at cluster level > > On 03/04/2012 03:22 PM, Ayal Baron wrote: > > > > > > ----- 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. > > enums are an issue, since we want to share them. > i agree services table seems the right approach, but actually, > booleans > are used here exactly because it seems services is the right long > term > approach, but we there is a lot of ground to cover before we know how > they would look like. > so boolean flags for the first few services, learning from these, > designing the bigger service models, and upgrading to it from the > cluster-with-flags seems (to me) the right path to take. >
+1 > > > > > >> > >>> > >>>> > >>>> 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 > > _______________________________________________ > 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
