Re: Proposal : For Auto-Prewarm.

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".
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(at)test-machine bin]$ ps -ef | grep prewarm
centos 21912 21905 0 08:39 ? 00:00:00 postgres: bgworker:
autoprewarm
[centos(at)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(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".
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(at)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 <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jun 5, 2017 at 7:58 AM, Rafia Sabih
> <rafia(dot)sabih(at)enterprisedb(dot)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(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

--

Regards,

Neha Sharma