Re: Proposal : For Auto-Prewarm.

Thanks, Amit,

On Fri, Jun 9, 2017 at 8:07 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)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