1 Dec
2013
1 Dec
'13
9:55 a.m.
------=_Part_21090047_1204116571.1385888119663
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
----- Original Message -----
> From: "Mike Kolesnik" <mkolesni@redhat.com>
> To: "Kanagaraj" <kmayilsa@redhat.com>
> Cc: "engine-devel" <engine-devel@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@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
------=_Part_21090047_1204116571.1385888119663
Content-Type: text/html; charset=utf-8
Content-Transfer-Encoding: quoted-printable
<html><body><div style=3D"font-family: times new roman, new york, times, se=
rif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id=
=3D"zwchr"><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5=
px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-de=
coration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>Fr=
om: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Kanagara=
j" <kmayilsa@redhat.com><br><b>Cc: </b>"engine-devel" <engine-deve=
l@ovirt.org><br><b>Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>S=
ubject: </b>Re: [Engine-devel] Using config values<br><div><br></div><div s=
tyle=3D"font-family: times new roman, new york, times, serif; font-size: 12=
pt; color: #000000"><hr id=3D"zwchr"><blockquote style=3D"border-left:2px s=
olid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal=
;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-se=
rif;font-size:12pt;">
=20
=20
=20
=20
Hi All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br=
></div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;p=
adding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decora=
tion:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br>
The are some issues arising in configurations whenever we move up on
the versions(3.3 =3D> 3.4), because of the way we store and
interpret them.<br><div><br></div>
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.<br=
>
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.<br><div><br></div>
Lets take an exmaple, "SupportForceCreateVG" - This is supported
from 3.1 onwards,<br><div><br></div>
If you look at config.sql, you will see following entries <br>
select fn_db_add_config_value('SupportForceCreateVG','false','3.0');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.1');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.2');
<br>
select fn_db_add_config_value('SupportForceCreateVG','true','3.3');<br>=
<div><br></div>
And in ConfigValues.java<br><div><br></div>
@TypeConverterAttribute(Boolean.class)<br>
@DefaultValueAttribute("false")<br>
SupportForceCreateVG,<br><div><br></div>
Now if there is 3.4 and 3.5, the user needs to add 2 more entries,
which i feel is redundant.<br><div><br></div>
Instead we can make <br><div><br></div>
@TypeConverterAttribute(Boolean.class)<br>
@DefaultValueAttribute("true")<br>
SupportForceCreateVG,<br><div><br></div>
and have only the following in config.sql<br>
select fn_db_add_config_value('SupportForceCreateVG','false','3.0');<br=
><div><br></div>
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.</blockquote><div><br></div><div>This has already bee=
n implemented, there are many "feature supported" configurations working li=
ke this (for example GlusterSupport).<br></div><div><br></div><div>I think =
a more interesting approach would be to move these out of the DB since thes=
e values don't really hav e a reson to be there.<br></div><div>Since the en=
tire thing is abstracted by "Gluster/FeatureSupported" classes then we can =
easily change mechanism (of course whatever code is not using it can be eas=
ily converted to use the mechanism)<br></div><div><br></div><div>For exampl=
e a simple enum could do the trick:<br></div><div>-------------------------=
------------ EXAMPLE -------------------------------------<br></div><div>/*=
*<br> * Convenience class to check if a gluster feature is supported o=
r not in any given version.<br><br> * Methods should be named by=
feature and accept version to check against.<br> */<br>public class G=
lusterFeatureSupported {</div><div> /**<br> &n=
bsp; * @param version<br> * =
Compatibility version to c=
heck for.<br> * @return <code>true</code&g=
t; if gluster support is enabled, <code>false</code> if it's no=
t.<br> */<br> public static boole=
an gluster(Version version) {<br> =
return SupportedFeatures.GLUSTER.isSupportedOn(version);<br> &n=
bsp; }<br><div><br></div> /**<br> =
* @param version<br> * &nbs=
p; Compatibility version to check for.<=
br> * @return <code>true</code> if glus=
ter heavyweight refresh is enabled, <code>false</code> if it's =
not.<br> */<br> public static boo=
lean refreshHeavyWeight(Version version) {<br> =
; return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(ve=
rsion);<br> }<br></div><div><br></div><div> &n=
bsp; /* More methods... */</div><div><br> enum SupportedF=
eatures {<br> GLUSTER(Version.v3_=
0),<br> REFRESH_HEAVYWEIGHT(Versi=
on.v3_0, Version.v3_1),</div><div> =
; /* More members */;<br><div><br></div> =
; private Set<Version> unsupportedVersions =3D new HashSet<V=
ersion>();<br><div><br></div> =
private SupportedFeatures(Version... versions) {<br> =
; unsupportedVersions.addAll(Arra=
ys.asList(versions));<br> }<br><d=
iv><br></div> public boolean isSu=
pportedOn(Version version) {<br> &=
nbsp; return !unsupportedVersions.contains(version);<br>&=
nbsp; }<br> }</div><d=
iv>------------------------------------- END EXAMPLE ----------------------=
---------------</div><div><br></div><div>Thoughts?</div></div></blockquote>=
<div><br>unless i didn't understand something, this is not good,<br></div><=
div>this should stay configurable by the users,<br></div><div>for example i=
f some user experience some issues with a feature and want to turn it off/c=
hange the values.. </div><div>(not all version configuration are boolean, s=
ome are different values to different versions, like cpu-list)<br></div><bl=
ockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padding-lef=
t:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;=
font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div style=3D"font-=
family: times new roman, new york, times, serif; font-size: 12pt; color: #0=
00000"><div><br></div><div><br></div><div>Regards,<br></div><div>Mike<br></=
div><blockquote style=3D"border-left:2px solid #1010FF;margin-left:5px;padd=
ing-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoratio=
n:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br>
Please share your thoughts on this.<br><div><br></div>
Thanks,<br>
Kanagaraj<br><div><br></div></blockquote></div><br>____________________=
___________________________<br>Engine-devel mailing list<br>Engine-devel@ov=
irt.org<br>http://lists.ovirt.org/mailman/listinfo/engine-devel<br></blockq=
uote><div><br></div></div></body></html>
------=_Part_21090047_1204116571.1385888119663--