Re: [Engine-devel] Using config values
On 12/01/2013 11:58 AM, Eli Mesika wrote: - Original Message - From: "Omer Frenkel" To: "Eli Mesika" Cc: "Dusmant Kumar Pati" , "engine-devel" Sent: Sunday, December 1, 2013 11:02:46 AM Subject: Re: [Engine-devel] Using config values - Original Message - From: "Eli Mesika" To: "Dusmant Kumar Pati" Cc: "engine-devel" Sent: Saturday, November 30, 2013 10:58:35 PM Subject: Re: [Engine-devel] Using config values - Original Message - From: "Dusmant Kumar Pati" To: "Kanagaraj" , "engine-devel" Sent: Friday, November 29, 2013 1:40:09 PM Subject: Re: [Engine-devel] Using config values On 11/29/2013 01:46 PM, Kanagaraj wrote: Hi All, 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. Please share your thoughts on this. Hi First of all I think its a good idea I have 2 questions 1) Which value will be stored as default in the java class for configuration values that are not a boolean, that represents if a feature is active or not. Is that the latest version value ? sounds not obvious to me i guess this will have to have a configuration values for each version in the db. 2) There are some configuration values that are exposed to the user via the engine-config tool, how this will work, we can not remove the entries their since the user may change and override those values. in your suggestion below, there is the same issue, if user want to change the 3.3 value, engine config will fail with "No such entry" because 3.3 will not be in the db.. so if we are not fixing this, i think using the current implementation is good enough, In this case engine-config will add this key with the given value for that version, so , I see no problem in that... In addition the getConfigValue will lookup for version matching only the first time, so , if it was given a 3.4 version for key K and found a matching only for 3.2 , it will add to the cache K with the same value for version 3.4 I don't think 3.4 should inherit from 3.2, only from the default value. i do think the code knowing if it should ask by cluster or dc level, or at 'general' level is wrong, and asking by cluster level should fetch the 'general' level before going to code level default. ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Using config values
On 12/01/2013 11:23 AM, Moti Asayag wrote: - Original Message - From: "Omer Frenkel" To: "Mike Kolesnik" Cc: "engine-devel" Sent: Sunday, December 1, 2013 11:09:32 AM Subject: Re: [Engine-devel] Using config values From: "Mike Kolesnik" To: "Omer Frenkel" Cc: "Kanagaraj" , "engine-devel" Sent: Sunday, December 1, 2013 11:01:50 AM Subject: Re: [Engine-devel] Using config values From: "Mike Kolesnik" To: "Kanagaraj" Cc: "engine-devel" Sent: Sunday, December 1, 2013 8:08:42 AM Subject: Re: [Engine-devel] Using config values 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. * Methods should be named by feature and accept version to check against. */ public class GlusterFeatureSupported { /** * @param version * Compatibility version to check for. * @return true if gluster support is enabled, false if it's not. */ public static boolean gluster(Version version) { return SupportedFeatures.GLUSTER.isSupportedOn(version); } /** * @param version * Compatibility version to check for. * @return true if gluster heavyweight refresh is enabled, false 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 unsupportedVersions = new HashSet(); 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) This is for API level compatibility. If VDSM doesn't support for example hot plug in 3.1 then the user can't just decide that it does and change it. Also, this is not changeable by user since it's not exposed by engine-config (nor should it be). some are exposed This is strictly for the engine-VDSM API compatibility, not for other configs which are version specif
Re: [Engine-devel] Using config values
- Original Message - > From: "Omer Frenkel" > To: "Eli Mesika" > Cc: "Dusmant Kumar Pati" , "engine-devel" > > Sent: Sunday, December 1, 2013 11:02:46 AM > Subject: Re: [Engine-devel] Using config values > > > > - Original Message - > > From: "Eli Mesika" > > To: "Dusmant Kumar Pati" > > Cc: "engine-devel" > > Sent: Saturday, November 30, 2013 10:58:35 PM > > Subject: Re: [Engine-devel] Using config values > > > > > > > > - Original Message - > > > From: "Dusmant Kumar Pati" > > > To: "Kanagaraj" , "engine-devel" > > > > > > Sent: Friday, November 29, 2013 1:40:09 PM > > > Subject: Re: [Engine-devel] Using config values > > > > > > On 11/29/2013 01:46 PM, Kanagaraj wrote: > > > > > > > > > Hi All, > > > > > > 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. > > > > > > Please share your thoughts on this. > > > > Hi > > > > First of all I think its a good idea > > I have 2 questions > > > > 1) Which value will be stored as default in the java class for > > configuration > > values that are not a boolean, that represents if a feature is active or > > not. > >Is that the latest version value ? sounds not obvious to me > > > > i guess this will have to have a configuration values for each version in the > db. > > > 2) There are some configuration values that are exposed to the user via the > > engine-config tool, how this will work, we can not remove the entries their > > since the user may change and override those values. > > > > in your suggestion below, there is the same issue, > if user want to change the 3.3 value, engine config will fail with "No such > entry" > because 3.3 will not be in the db.. > so if we are not fixing this, i think using the current implementation is > good enough, In this case engine-config will add this key with the given value for that version, so , I see no problem in that... In addition the getConfigValue will lookup for version matching only the first time, so , if it was given a 3.4 version for key K and found a matching only for 3.2 , it will add to the cache K with the same value for version 3.4 > no need to add logic, just to convert curr
Re: [Engine-devel] Using config values
Regards, Mike - Original Message - > - Original Message - > > From: "Mike Kolesnik" > > > To: "Omer Frenkel" > > > Cc: "Kanagaraj" , "engine-devel" > > > > > Sent: Sunday, December 1, 2013 11:01:50 AM > > > Subject: Re: [Engine-devel] Using config values > > > - Original Message - > > > > - Original Message - > > > > > > > From: "Mike Kolesnik" > > > > > > > > > > To: "Kanagaraj" > > > > > > > > > > Cc: "engine-devel" > > > > > > > > > > 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 > >
Re: [Engine-devel] Using config values
- Original Message - > From: "Omer Frenkel" > To: "Mike Kolesnik" > Cc: "engine-devel" > Sent: Sunday, December 1, 2013 11:09:32 AM > Subject: Re: [Engine-devel] Using config values > > > > > > > From: "Mike Kolesnik" > To: "Omer Frenkel" > Cc: "Kanagaraj" , "engine-devel" > > Sent: Sunday, December 1, 2013 11:01:50 AM > Subject: Re: [Engine-devel] Using config values > > > > > > > > > > > From: "Mike Kolesnik" > To: "Kanagaraj" > Cc: "engine-devel" > Sent: Sunday, December 1, 2013 8:08:42 AM > Subject: Re: [Engine-devel] Using config values > > > > > 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. > * Methods should be named by feature and accept version to check against. > */ > public class GlusterFeatureSupported { > /** > * @param version > * Compatibility version to check for. > * @return true if gluster support is enabled, false > if it's not. > */ > public static boolean gluster(Version version) { > return SupportedFeatures.GLUSTER.isSupportedOn(version); > } > > /** > * @param version > * Compatibility version to check for. > * @return true if gluster heavyweight refresh is enabled, > false 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 unsupportedVersions = new HashSet(); > > 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 understa
Re: [Engine-devel] Using config values
- Original Message - > From: "Mike Kolesnik" > To: "Omer Frenkel" > Cc: "Kanagaraj" , "engine-devel" > > Sent: Sunday, December 1, 2013 11:01:50 AM > Subject: Re: [Engine-devel] Using config values > - Original Message - > > - Original Message - > > > > From: "Mike Kolesnik" > > > > > > To: "Kanagaraj" > > > > > > Cc: "engine-devel" > > > > > > 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 > > > - > > > > > > /*
Re: [Engine-devel] Using config values
- Original Message - > From: "Eli Mesika" > To: "Dusmant Kumar Pati" > Cc: "engine-devel" > Sent: Saturday, November 30, 2013 10:58:35 PM > Subject: Re: [Engine-devel] Using config values > > > > - Original Message - > > From: "Dusmant Kumar Pati" > > To: "Kanagaraj" , "engine-devel" > > > > Sent: Friday, November 29, 2013 1:40:09 PM > > Subject: Re: [Engine-devel] Using config values > > > > On 11/29/2013 01:46 PM, Kanagaraj wrote: > > > > > > Hi All, > > > > 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. > > > > Please share your thoughts on this. > > Hi > > First of all I think its a good idea > I have 2 questions > > 1) Which value will be stored as default in the java class for configuration > values that are not a boolean, that represents if a feature is active or > not. >Is that the latest version value ? sounds not obvious to me > i guess this will have to have a configuration values for each version in the db. > 2) There are some configuration values that are exposed to the user via the > engine-config tool, how this will work, we can not remove the entries their > since the user may change and override those values. > in your suggestion below, there is the same issue, if user want to change the 3.3 value, engine config will fail with "No such entry" because 3.3 will not be in the db.. so if we are not fixing this, i think using the current implementation is good enough, no need to add logic, just to convert current options that are not in this format, to use this logic. > I have a different suggestion: > Default value will stay as is , meaning , it will reflect the value that > should be used to keep the application running correctly if a value is not > found in DB (which should not occur) > > Code of getting configuration value (getConfigValue(,) will be > changed to get the closest version value to the given one. > For example , if a 3.4 version is given for a given and we have in DB > just values for 3.0 and 3.1 , the 3.1 value is returned. > I prefer this solution since it makes the config.sql file self documented , > showing only value changes and in which version each change occurred. > > To implement that, we should add this mechanism to the current code that > caches the DB content and as I see that it should be a simple change. > engine-config should be modified such that if the user change a value, this > value will be inserted to the database with the current release if not > exists and then the mechanism described above will get this value > > Example: > > VdsFenceType lists all the support
Re: [Engine-devel] Using config values
- Original Message - > - Original Message - > > From: "Mike Kolesnik" > > > To: "Kanagaraj" > > > Cc: "engine-devel" > > > 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. > > > * Methods should be named by feature and accept version to check against. > > > */ > > > public class GlusterFeatureSupported { > > > /** > > > * @param version > > > * Compatibility version to check for. > > > * @return true if gluster support is enabled, > > false > > if it's not. > > > */ > > > public static boolean gluster(Version version) { > > > return SupportedFeatures.GLUSTER.isSupportedOn(version); > > > } > > > /** > > > * @param version > > > * Compatibility version to check for. > > > * @return true if gluster heavyweight refresh is enabled, > > false if it's not. > > > */ > > > public static boolean refreshHeavyWeight(Version version) { > > > return SupportedFeatures.REFRESH_HEAVYWEIGHT.isSupportedOn(version); > > > } > > > /* More methods... */ > > >
Re: [Engine-devel] Using config values
- Original Message - > From: "Mike Kolesnik" > To: "Kanagaraj" > Cc: "engine-devel" > 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. > * Methods should be named by feature and accept version to check against. > */ > public class GlusterFeatureSupported { > /** > * @param version > * Compatibility version to check for. > * @return true if gluster support is enabled, false > if it's not. > */ > public static boolean gluster(Version version) { > return SupportedFeatures.GLUSTER.isSupportedOn(version); > } > /** > * @param version > * Compatibility version to check for. > * @return true if gluster heavyweight refresh is enabled, > false 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 unsupportedVersions = new HashSet(); > 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 ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
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. * Methods should be named by feature and accept version to check against. */ public class GlusterFeatureSupported { /** * @param version * Compatibility version to check for. * @return true if gluster support is enabled, false if it's not. */ public static boolean gluster(Version version) { return SupportedFeatures.GLUSTER.isSupportedOn(version); } /** * @param version * Compatibility version to check for. * @return true if gluster heavyweight refresh is enabled, false 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 unsupportedVersions = new HashSet(); private SupportedFeatures(Version... versions) { unsupportedVersions.addAll(Arrays.asList(versions)); } public boolean isSupportedOn(Version version) { return !unsupportedVersions.contains(version); } } - END EXAMPLE - Thoughts? 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
Re: [Engine-devel] Using config values
- Original Message - > From: "Dusmant Kumar Pati" > To: "Kanagaraj" , "engine-devel" > Sent: Friday, November 29, 2013 1:40:09 PM > Subject: Re: [Engine-devel] Using config values > > On 11/29/2013 01:46 PM, Kanagaraj wrote: > > > Hi All, > > 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. > > Please share your thoughts on this. Hi First of all I think its a good idea I have 2 questions 1) Which value will be stored as default in the java class for configuration values that are not a boolean, that represents if a feature is active or not. Is that the latest version value ? sounds not obvious to me 2) There are some configuration values that are exposed to the user via the engine-config tool, how this will work, we can not remove the entries their since the user may change and override those values. I have a different suggestion: Default value will stay as is , meaning , it will reflect the value that should be used to keep the application running correctly if a value is not found in DB (which should not occur) Code of getting configuration value (getConfigValue(,) will be changed to get the closest version value to the given one. For example , if a 3.4 version is given for a given and we have in DB just values for 3.0 and 3.1 , the 3.1 value is returned. I prefer this solution since it makes the config.sql file self documented , showing only value changes and in which version each change occurred. To implement that, we should add this mechanism to the current code that caches the DB content and as I see that it should be a simple change. engine-config should be modified such that if the user change a value, this value will be inserted to the database with the current release if not exists and then the mechanism described above will get this value Example: VdsFenceType lists all the supported fencing agents for power management , it currently has the following settings option_value | version -+- alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs | 3.0 alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs | 3.1 apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti | 3.2 apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti | 3.3 In the proposed solution, we will have option_value | version -+- alom,apc,bladecenter,drac5,eps,ilo,ilo3,ipmilan,rsa,rsb,wti,cisco_ucs | 3.0 apc,apc_snmp,bladecenter,cisco_ucs,drac5,eps,ilo,ilo2,ilo3,ilo4,ipmilan,rsa,rsb,wti | 3.2 This is clear and docu
Re: [Engine-devel] Using config values
On 11/29/2013 01:46 PM, Kanagaraj wrote: Hi All, 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. Please share your thoughts on this. Thanks, Kanagaraj ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel I think, this is a good suggestion... ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
Re: [Engine-devel] Using config values
On 11/29/2013 01:46 PM, Kanagaraj wrote: Hi All, 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. Please share your thoughts on this. +1 Thanks, Kanagaraj ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
[Engine-devel] Using config values
Hi All, 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. Please share your thoughts on this. Thanks, Kanagaraj ___ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel