Re: Proposal : For Auto-Prewarm.

On Tue, Jun 6, 2017 at 3:48 PM, Neha Sharma
<neha(dot)sharma(at)enterprisedb(dot)com> wrote:
> Hi,
>
> I have been testing this feature for a while and below are the observations
> for few scenarios.
>
> Observation:
> scenario 1: If we set pg_prewarm.dump_interval = -1.0,we get an additional
> warning message in logfile and instead of ending the task of auto-dump it
> executes successfully.
> [centos(at)test-machine bin]$ more logfile
> 2017-06-06 08:39:53.127 GMT [21905] WARNING: invalid value for parameter
> "pg_prewarm.dump_interval": "-1.0"
> 2017-06-06 08:39:53.127 GMT [21905] HINT: Valid units for this parameter
> are "ms", "s", "min", "h", and "d".
>
> postgres=# show pg_prewarm.dump_interval;
> pg_prewarm.dump_interval
> --------------------------
> 5min
> (1 row)
>
> scenario 2: If we set pg_prewarm.dump_interval = 0.0,we get an additional
> warning message in logfile and the message states that the task was started
> and the worker thread it is also active,but the dump_interval duration is
> set to default 5 min (300 sec) instead of 0.
>
> [centos(at)test-machine bin]$ ps -ef | grep prewarm
> centos 21980 21973 0 08:54 ? 00:00:00 postgres: bgworker:
> autoprewarm
>
> [centos(at)test-machine bin]$ more logfile
> 2017-06-06 09:20:52.436 GMT [22223] WARNING: invalid value for parameter
> "pg_prewarm.dump_interval": "0.0"
> 2017-06-06 09:20:52.436 GMT [22223] HINT: Valid units for this parameter
> are "ms", "s", "min", "h", and "d".
> postgres=# show pg_prewarm.dump_interval;
> pg_prewarm.dump_interval
> --------------------------
> 5min
> (1 row)

dump_interval is int type custom GUC variable so passing floating
value is illegal here, but the reason we are only throwing a warning
and setting it to default 5 mins is that of existing behavior

@define_custom_variable

/*
* Assign the string value(s) stored in the placeholder to the real
* variable. Essentially, we need to duplicate all the active and stacked
* values, but with appropriate validation and datatype adjustment.
*
* If an assignment fails, we report a WARNING and keep going. We don't
* want to throw ERROR for bad values, because it'd bollix the add-on
* module that's presumably halfway through getting loaded. In such cases
* the default or previous state will become active instead.
*/

/* First, apply the reset value if any */
if (pHolder->reset_val)
(void) set_config_option(name, pHolder->reset_val,
pHolder->gen.reset_scontext,
pHolder->gen.reset_source,
GUC_ACTION_SET, true, WARNING, false);

I think this should be fine as it is defined behavior for all of the
add-on modules. Please let me know if you have any suggestions.