<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>From: </b>"Mike Kolesnik" <mkolesni@redhat.com><br><b>To: </b>"Kanagaraj" <kmayilsa@redhat.com><br><b>Cc: </b>"engine-devel" <engine-devel@ovirt.org><br><b>Sent: </b>Sunday, December 1, 2013 8:08:42 AM<br><b>Subject: </b>Re: [Engine-devel] Using config values<br><div><br></div><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><hr id="zwchr"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
Hi All,</blockquote><div><br></div><div>Hi Kanagaraj,<br></div><div><br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration: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 => 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 been implemented, there are many "feature supported" configurations working like 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 these values don't really hav e a reson to be there.<br></div><div>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)<br></div><div><br></div><div>For example 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 or not in any given version.<br><br> * Methods should be named by feature and accept version to check against.<br> */<br>public class GlusterFeatureSupported {</div><div> /**<br> * @param version<br> * Compatibility version to check for.<br> * @return <code>true</code> if gluster support is enabled, <code>false</code> if it's not.<br> */<br> public static boolean gluster(Version version) {<br> return SupportedFeatures.GLUSTER.isSupportedOn(version);<br> }<br><div><br></div> /**<br> * @param version<br> * Compatibility version to check for.<br> * @return <code>true</code> if gluster heavyweight refresh is enabled, <code>false</code> if it's not.<br> */<br> public static boolean refreshHeavyWeight(Version version) {<br> return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version);<br> }<br></div><div><br></div><div> /* More methods... */</div><div><br> enum SupportedFeatures {<br> GLUSTER(Version.v3_0),<br> REFRESH_HEAVYWEIGHT(Version.v3_0, Version.v3_1),</div><div> /* More members */;<br><div><br></div> private Set<Version> unsupportedVersions = new HashSet<Version>();<br><div><br></div> private SupportedFeatures(Version... versions) {<br> unsupportedVersions.addAll(Arrays.asList(versions));<br> }<br><div><br></div> public boolean isSupportedOn(Version version) {<br> return !unsupportedVersions.contains(version);<br> }<br> }</div><div>------------------------------------- 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 if some user experience some issues with a feature and want to turn it off/change the values.. </div><div>(not all version configuration are boolean, some are different values to different versions, like cpu-list)<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><div>Regards,<br></div><div>Mike<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration: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@ovirt.org<br>http://lists.ovirt.org/mailman/listinfo/engine-devel<br></blockquote><div><br></div></div></body></html>