Thanks, Amit, On Fri, Jun 9, 2017 at 8:07 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> Few comments on the latest patch:
> +
> + /* Prewarm buffer. */
> + buf = ReadBufferExtended(rel, blk->forknum, blk->blocknum, RBM_NORMAL,
> + NULL);
> + if (BufferIsValid(buf))
> + ReleaseBuffer(buf);
> +
> + old_blk = blk;
> + ++pos;
>
> You are incrementing position at different places in this loop. I
> think you can do it once at the start of the loop.
Fixed now moved all of ++pos to one place now.
> 2.
> +dump_now(bool is_bgworker)
> {
> ..
> + fd = OpenTransientFile(transient_dump_file_path,
> + O_CREAT | O_WRONLY | O_TRUNC, 0666);
>
> +prewarm_buffer_pool(void)
> {
> ..
> + file = AllocateFile(AUTOPREWARM_FILE, PG_BINARY_R);
>
> During prewarm, you seem to be using binary mode to open a file
> whereas during dump binary flag is not passed. Is there a reason
> for such a difference?
>
-- Sorry fixed now, both use binary.
>
> 3.
> + ereport(LOG,
> + (errmsg("saved metadata info of %d blocks", num_blocks)));
>
> It doesn't seem like a good idea to log this info at each dump
> interval. How about making this as a DEBUG1 message?
-- Fixed, made it as DEBUG1 along with another message "autoprewarm load
task ended" message.
--
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com
autoprewarm_13.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
