[Engine-devel] Using config values

[Omer Frenkel]

----- Original Message -----

> From: "Mike Kolesnik" <mkolesni at redhat.com>
> To: "Kanagaraj" <kmayilsa at redhat.com>
> Cc: "engine-devel" <engine-devel at ovirt.org>
> Sent: Sunday, December 1, 2013 8:08:42 AM
> Subject: Re: [Engine-devel] Using config values

> ----- Original Message -----

> > Hi All,
> 
> Hi Kanagaraj,

> > The are some issues arising in configurations whenever we move up on the
> > versions(3.3 => 3.4), because of the way we store and interpret them.
> 

> > Whenever there is a new cluster level, you will need to add a new entry for
> > all(most) of the configuration. Mostly a copy paste if you see from 3.2 to
> > 3.3, except some CPU/PM type related configurations.
> 
> > Better option would be to have the defaul config value in ConfigValues.java
> > and the overrides will go to config.sql. In this approach you don't need a
> > new entries to config.sql when there is a new cluster level.
> 

> > Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1
> > onwards,
> 

> > If you look at config.sql, you will see following entries
> 
> > select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
> 
> > select fn_db_add_config_value('SupportForceCreateVG','true','3.1');
> 
> > select fn_db_add_config_value('SupportForceCreateVG','true','3.2');
> 
> > select fn_db_add_config_value('SupportForceCreateVG','true','3.3');
> 

> > And in ConfigValues.java
> 

> > @TypeConverterAttribute(Boolean.class)
> 
> > @DefaultValueAttribute("false")
> 
> > SupportForceCreateVG,
> 

> > Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which i
> > feel is redundant.
> 

> > Instead we can make
> 

> > @TypeConverterAttribute(Boolean.class)
> 
> > @DefaultValueAttribute("true")
> 
> > SupportForceCreateVG,
> 

> > and have only the following in config.sql
> 
> > select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
> 

> > if a particular value(for a specific cluster level) is not found in
> > Config.sql, the fallback is to use the value available in
> > ConfigValues.java.
> 
> This has already been implemented, there are many "feature supported"
> configurations working like this (for example GlusterSupport).

> I think a more interesting approach would be to move these out of the DB
> since these values don't really hav e a reson to be there.
> Since the entire thing is abstracted by "Gluster/FeatureSupported" classes
> then we can easily change mechanism (of course whatever code is not using it
> can be easily converted to use the mechanism)

> For example a simple enum could do the trick:
> ------------------------------------- EXAMPLE
> -------------------------------------
> /**
> * Convenience class to check if a gluster feature is supported or not in any
> given version.<br>
> * Methods should be named by feature and accept version to check against.
> */
> public class GlusterFeatureSupported {
> /**
> * @param version
> * Compatibility version to check for.
> * @return <code>true</code> if gluster support is enabled, <code>false</code>
> if it's not.
> */
> public static boolean gluster(Version version) {
> return SupportedFeatures.GLUSTER.isSupportedOn(version);
> }

> /**
> * @param version
> * Compatibility version to check for.
> * @return <code>true</code> if gluster heavyweight refresh is enabled,
> <code>false</code> if it's not.
> */
> public static boolean refreshHeavyWeight(Version version) {
> return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);
> }

> /* More methods... */

> enum SupportedFeatures {
> GLUSTER(Version.v3_0),
> REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),
> /* More members */;

> private Set<Version> unsupportedVersions = new HashSet<Version>();

> private SupportedFeatures(Version... versions) {
> unsupportedVersions.addAll(Arrays.asList(versions));
> }

> public boolean isSupportedOn(Version version) {
> return !unsupportedVersions.contains(version);
> }
> }
> ------------------------------------- END EXAMPLE
> -------------------------------------

> Thoughts?

unless i didn't understand something, this is not good, 
this should stay configurable by the users, 
for example if some user experience some issues with a feature and want to turn it off/change the values.. 
(not all version configuration are boolean, some are different values to different versions, like cpu-list) 

> Regards,
> Mike

> > Please share your thoughts on this.
> 

> > Thanks,
> 
> > Kanagaraj
> 

> _______________________________________________
> Engine-devel mailing list
> Engine-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20131201/0b36ec9b/attachment-0001.html>