[Engine-devel] [RFC] ovirt-engine - vdc_config default options

[Alon Bar-Lev]

----- Original Message -----
> From: "Livnat Peer" <lpeer at redhat.com>
> To: "Alon Bar-Lev" <alonbl at redhat.com>
> Cc: engine-devel at ovirt.org
> Sent: Monday, September 10, 2012 8:55:32 AM
> Subject: Re: [Engine-devel] [RFC] ovirt-engine - vdc_config default options
> 
> On 10/09/12 00:58, Alon Bar-Lev wrote:
> > Hello All,
> > 
> > I would like to discuss the method we use to manage default
> > options. I believe it can be significantly simplified.
> > 
> > Please read though and comment.
> > 
> > Thank you,
> > Alon Bar-Lev
> > 
> > ---
> > 
> > CURRENT STATE
> > 
> > Most options are located in three different locations.
> > 
> > Let's explain by example... The
> > FenceQuietTimeBetweenOperationsInSec option.
> > 
> > ---
> > ConfigValues.java
> > ---
> > public enum ConfigValues {
> > <snip>
> >     @Reloadable
> >     @TypeConverterAttribute(Integer.class)
> >     @DefaultValueAttribute("180")
> >     FenceQuietTimeBetweenOperationsInSec(30),
> > <snip>
> > }
> > ---
> > 
> > ---
> > 0000_config.sql
> > ---
> > <snip>
> > select
> > fn_db_add_config_value('FenceQuietTimeBetweenOperationsInSec','180','general');
> > <snip>
> > ---
> > 
> > ---
> > engine-config.properties
> > ---
> > <snip>
> > FenceQuietTimeBetweenOperationsInSec.type=Integer
> > FenceQuietTimeBetweenOperationsInSec.validValues=60..600
> > FenceQuietTimeBetweenOperationsInSec.description="Fence quiet time
> > between operations (in seconds)"
> > FenceQuietTimeBetweenOperationsInSec.alternateKey=Fence_Quiet_Time
> > <snip>
> > ---
> > 
> > ConfigValues.java is the base, it defines all options and
> > appropriate metadata as annotations. It defines the option name,
> > type, default value, and if reloadable.
> > 
> > 0000_config.sql adds all parameters into the database using their
> > default value, we actually duplicate the parameter name and
> > default value into the sql script. Please note that even if we do
> > not add the parameter to the database the engine infrastructure
> > will use the default value specify at ConfigValues.
> > 
> > engine-config.properties is used as an interface to engine-config
> > utility, a command-line utility that can manipulate engine
> > options. engine-config manages only options that are specified in
> > this property files. Property files specifies user friendly
> > description, valid values, alias, but duplicate the option name
> > and type that already specified in ConfigValues. engine-config
> > will only manage options that exists in database.
> > 
> > QUESTIONS
> > 
> > 1. Why do we store default values in database?
> > 
> 
> Our intention so far was to remove default values from the code.
> 
> The values in the data base are not default values but the values of
> the
> properties and we keep them per version were we need different values
> per version.
> 
> The obvious advantage of keeping them in the DB vs. in code is they
> can
> be changed with no compilation required and today you don't need to
> restart the application for changing some of them (WIP).

The above statement is true if you use database or any other format, such as XML file for the metadata.

As long as there is no duplication between code and metadata storage, and as long as the default values are not stored within the options tables, so that after upgrade the new defaults are in effect unless explicitly overridden by user.

Let's assume we would like to avoid compilation. What is the benefit in storing the metadata within the database and not in plain text file / XML file / properties file?

> > From what I managed to gather, we store default values in database
> > under the assumption that we need to keep defaults when we upgrade
> > from one version to another.
> > 
> > However, in most cases when upgrading an application the new
> > defaults should apply as long as they were not overridden by user.
> > Keeping old default as a new value is an exception that can be
> > taken care of during the upgrade process for selected options.
> > 
> > 2. Why do we keep properties file?
> 
> 
> The properties file primary use was to add translation/description
> for
> each property. The reason we store it in the file was that at some
> point
> we'll support multilingual description for each property.

Usually, multilingual has the "C" (latin) within code, while overriding the "other" lingual within language support pack. So that application without any language support pack will speak latin.

The properties file also contains non multilingual items: restrictions and alias, so if the propose is to be lingual, it should contain only description.

> > 
> > In practice we could specify the metadata within the property files
> > as annotations in ConfigValues.java. So why do we actually need
> > the properties file?
> > 
> > From what I managed to gather, we use the property file as an
> > interface to support personal, to allow adding/removing options
> > exposed to the engine-config utility. An addition of option to the
> > property file exposes it.
> > 
> > SUGGESTED STATE
> > 
> > Establish a single place to manage options.
> > Do not store default values in database.
> > 
> > Provide alternate method of exposing internal options to
> > engine-config utility.
> > 
> 
> Maintaining that in a single place would be great.
> 
> 1. Were do you store the description of each property (and optionally
> support multilingual)?

Description (non C multi-lingual) should be separate from other (operative) metadata.

The C (latin) description should be placed within core product metadata. Each other supported linual should be within its own separate file.

This way you install ovirt-engine-lang-pack-es and application speaks es without change in database or any of the installed files.

The search path for description is (1) use application locale, and if not found (2) use the C locale. So when no description available at language pack, the core C is used.

> 2. Where do you store value per version?

When user set value it will be in database no change in that.

The question is where we keep the metadata.

We can keep the metadata within ConfigValues as annotations.
We can keep the metadata within XML or whatever format.
We can keep the metadata in *SEPARATE* table in database.

The benefit of using XML is that you can have one common metadata and one per product variant. So that metadata search path can be (1) product variant, and if not found (2) use the common metadata.

Same logic can be used within database only that it will be somewhat more complex to implement.
 
> 3. I personally find holding values in code cumbersome, usually you
> end
> up with adding option to override the values by entries in the data
> base.....

Right,
This is what default is all about.
However, when introducing a new variable, the database is left as-is.
When changing default value that was not overridden by user, the database is left as-is.
And... as time goes by, developers collect the common use case, to apply into next version, reducing the need to override defaults.

> I would like to have everything stored in the data base.
> We just need to do it (which we did not have time to do so far) and
> we
> need to think of solution for the description field,(multilingual
> support is typically done via properties file).

I am curios to read why database is better mechanism for static metadata than plain XML.

> 
> > ---
> > ConfigValues.java
> > ---
> > public enum ConfigValues {
> > <snip>
> >     @Reloadable
> >     @Public
> >     @TypeConverterAttribute(Integer.class)
> >     @Restriction.IntegerRange(60, 600)
> >     @DefaultValueAttribute("180")
> >     @Description("Fence quiet time between operations (in
> >     seconds)")
> >     FenceQuietTimeBetweenOperationsInSec(30),
> > <snip>
> > }
> > ---
> > 
> > BENEFITS
> > 
> > No duplication of option name, type, default value, etc... between
> > multiple files. One place to role them all.
> > 
> > Simpler upgrade sequence, in most cases as we do want the new
> > defaults to apply.
> > 
> > METHOD
> > 
> > 1. Add @Public annotation for ConfigValues, to expose options to
> > engine-config instead of the properties file.
> > 
> > 2. Add @Restrict annotation for ConfigValues, instead of
> > validValues of properties file.
> > 
> > 3. Add @Description annotation for ConfigValues, instead of
> > description of properties file.
> > 
> > 4. Add engine-config --internal parameter to allow get/set/list of
> > none @Public option, instead of the need to update the properties
> > file.
> > 
> > 5. Modify enigne-config [set] to add option if does not exist in
> > database and does not match default value.
> > 
> > 6. Modify engine-config [set] to delete option if value matches
> > default value.
> > 
> > 7. Modify engine-config [get] to retrieve default from ConfigValues
> > if value is missing from database.
> > 
> > 8. Create upgrade script which deletes all options with value that
> > matches the default from the database.
> > 
> > IMPLICATIONS
> > 
> > The option alias is removed. Can be added at later time if required
> > using own @Alias annotation. Not sure it is actually required.
> > 
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> > 
> 
>