Re: [HACKERS] Proposal : For Auto-Prewarm.

On Tue, Jun 6, 2017 at 3:48 PM, Neha Sharma
<neha.sharma@enterprisedb.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@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@test-machine bin]$ ps -ef | grep prewarm
> centos   21980 21973  0 08:54 ?        00:00:00 postgres: bgworker:
> autoprewarm
>
> [centos@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
duplicateall the active and stacked    * values, but with appropriate validation and datatype adjustment.    *    * If
anassignment fails, we report a WARNING and keep going.  We don't    * want to throw ERROR for bad values, because it'd
bollixthe add-on    * module that's presumably halfway through getting loaded.  In such cases    * the default or
previousstate 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.