From: | Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com> |
---|---|
To: | Beena Emerson <memissemerson(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Proposal : For Auto-Prewarm. |
Date: | 2017-02-07 12:22:47 |
Message-ID: | CAD__Ouj57AdnkF=UZROn9=XxB3OaQLgSwzKSmryqu-41RS=pfg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks Beena,
On Tue, Feb 7, 2017 at 4:46 PM, Beena Emerson <memissemerson(at)gmail(dot)com> wrote:
> Few more comments:
>
> = Background worker messages:
>
> - Workers when launched, show messages like: "logical replication launcher
> started”, "autovacuum launcher started”. We should probably have a similar
> message to show that the pg_prewarm load and dump bgworker has started.
-- Thanks, I will add startup and shutdown message.
> - "auto pg_prewarm load: number of buffers to load x”, other messages show
> space before and after “:”, we should keep it consistent through out.
>
-- I think you are testing patch 03. The latest patch_04 have
corrected same. Can you please re-test it.
>
> = Action of -1.
> I thought we decided that interval value of -1 would mean that the auto
> prewarm worker will not be run at all. With current code, -1 is explained to
> mean it will not dump. I noticed that reloading with new option as -1 stops
> both the workers but restarting loads the data and then quits. Why does it
> allow loading with -1? Please explain this better in the documents.
>
-- '-1' means we do not want to dump at all. So we decide not to
continue with launched bgworker and it exits. As per your first
comment, if I register the startup and shutdown messages for auto
pg_prewarm I think it will look better. Will try to explain it in a
better way in documentation. The "auto pg_prewarm load" will not be
affected with dump_interval value. It will always start, load(prewarm)
and then exit.
>
> = launch_pg_prewarm_dump()
> =# SELECT launch_pg_prewarm_dump();
> launch_pg_prewarm_dump
> ------------------------
> 53552
> (1 row)
>
> $ ps -ef | grep 53552
> b_emers+ 53555 4391 0 16:21 pts/1 00:00:00 grep --color=auto 53552
-- If dump_interval = -1 "auto pg_prewarm dump" will exit immediately.
> = Function names
> - load_now could be better named as load_file, load_dumpfile or similar.
> - dump_now -> dump_buffer or better?
I did choose load_now and dump_now to indicate we are doing it
immediately as invoking them was based on a timer/state. Probably we
can improve that but dump_buffer, load_file may not be the right
replacement.
>
> = Corrupt file
> if the dump file is corrupted, the system crashes and the prewarm bgworkers
> are not restarted. This needs to be handled better.
>
> WARNING: terminating connection because of crash of another server process
> 2017-02-07 16:36:58.680 IST [54252] DETAIL: The postmaster has commanded
> this server process to roll back the current transaction and exit, because
> another server process exited abnormally and possibly corrupted shared
> memory
--- Can you please paste you autopgprewarm.save file, I edited the
file manually to some illegal entry but did not see any crash. Only
we failed to load as block number were invalid. Please share your
tests so that I can reproduce same.
> = Documentation
>
> I feel the documentation needs to be improved greatly.
>
> - The first para in pg_prewarm should mention the autoload feature too.
>
> - The new section should be named “The pg_prewarm autoload” or something
> better. "auto pg_prewarm bgworker” does not seem appropriate. The
> configuration parameter should also be listed out clearly like in auth-delay
> page. The new function launch_pg_prewarm_dump() should be listed under
> Functions.
-- Thanks I will try to improve the documentation. And, put more details there.
--
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Beena Emerson | 2017-02-07 12:35:28 | Re: Proposal : For Auto-Prewarm. |
Previous Message | Yugo Nagata | 2017-02-07 12:21:02 | Re: Postgres_fdw behaves oddly |