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". 2017-06-06 08:39:53.127 GMT [21905] LOG: listening on IPv6 address "::1", port 5432 2017-06-06 08:39:53.127 GMT [21905] LOG: listening on IPv4 address "127.0.0.1", port 5432 2017-06-06 08:39:53.130 GMT [21905] LOG: listening on Unix socket "/tmp/.s.PGSQL.5432" 2017-06-06 08:39:53.143 GMT [21906] LOG: database system was shut down at 2017-06-06 08:38:20 GMT 2017-06-06 08:39:53.155 GMT [21905] LOG: database system is ready to accept connections 2017-06-06 08:39:53.155 GMT [21912] LOG: autoprewarm has started [centos@test-machine bin]$ ps -ef | grep prewarm centos 21912 21905 0 08:39 ? 00:00:00 postgres: bgworker: autoprewarm [centos@test-machine bin]$ ./psql postgres psql (10beta1) Type "help" for help. 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". 2017-06-06 09:20:52.436 GMT [22223] LOG: listening on IPv6 address "::1", port 5432 2017-06-06 09:20:52.437 GMT [22223] LOG: listening on IPv4 address "127.0.0.1", port 5432 2017-06-06 09:20:52.439 GMT [22223] LOG: listening on Unix socket "/tmp/.s.PGSQL.5432" 2017-06-06 09:20:52.452 GMT [22224] LOG: database system was shut down at 2017-06-06 09:19:49 GMT 2017-06-06 09:20:52.455 GMT [22223] LOG: database system is ready to accept connections 2017-06-06 09:20:52.455 GMT [22230] LOG: autoprewarm has started [centos@test-machine bin]$ ./psql postgres psql (10beta1) Type "help" for help. postgres=# show pg_prewarm.dump_interval; pg_prewarm.dump_interval -------------------------- 5min (1 row) On Mon, Jun 5, 2017 at 8:06 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Mon, Jun 5, 2017 at 7:58 AM, Rafia Sabih > <rafia.sa...@enterprisedb.com> wrote: > > I had a look at the patch from stylistic/formatting point of view, > > please find the attached patch for the suggested modifications. > > Many of these seem worse, like these ones: > > - * Quit if we've reached records for another database. Unless the > + * Quit if we've reached records of another database. Unless the > > - * When we reach a new relation, close the old one. Note, > however, > - * that the previous try_relation_open may have failed, in which > case > - * rel will be NULL. > + * On reaching a new relation, close the old one. Note, that the > + * previous try_relation_open may have failed, in which case rel > will > + * be NULL. > > - * Try to open each new relation, but only once, when we first > - * encounter it. If it's been dropped, skip the associated > blocks. > + * Each relation is open only once at it's first encounter. If > it's > + * been dropped, skip the associated blocks. > > Others are better, like these: > > - (errmsg("could not continue autoprewarm worker is > already running under PID %d", > + (errmsg("autoprewarm worker is already running under PID > %d", > > - * Start of prewarm per-database worker. This will try to load blocks of > one > + * Start prewarm per-database worker, which will load blocks of one > > Others don't really seem better or worse, like: > > - * Register a per-database worker to load new database's block. > And > - * wait until they finish their job to launch next one. > + * Register a per-database worker to load new database's block. > Wait > + * until they finish their job to launch next one. > > IMHO, there's still a good bit of work needed here to make this sound > like American English. For example: > > - * It is a bgworker which automatically records information about > blocks > - * which were present in buffer pool before server shutdown and > then > - * prewarm the buffer pool upon server restart with those blocks. > + * It is a bgworker process that automatically records information > about > + * blocks which were present in buffer pool before server > shutdown and then > + * prewarms the buffer pool upon server restart with those blocks. > > This construction "It is a..." without a clear referent seems to be > standard in Indian English, but it looks wrong to English speakers > from other parts of the world, or at least to me. > > + * Since there could be at max one worker who could do a prewarm, > hence, > + * acquiring locks is not required before setting > skip_prewarm_on_restart. > > To me, adding a comma before hence looks like a significant > improvement, but the word hence itself seems out-of-place. Also, I'd > change "at max" to "at most" and maybe reword the sentence a little. > There's a lot of little things like this which I have tended be quite > strict about changing before commit; I occasionally wonder whether > it's really worth the effort. It's not really wrong, it just sounds > weird to me as an American. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Regards, Neha Sharma