Re: Proposal : For Auto-Prewarm.

From: Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal : For Auto-Prewarm.
Date: 2017-06-12 01:01:19
Message-ID: CAD__Oujd4sr_t_juqJ+HjeLtCegw7xe8M1hB_=vjre1OJfwfuQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
autoprewarm_13.patch application/octet-stream 35.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Yugo Nagata 2017-06-12 01:38:45 documentation typo in ALTER TABLE example
Previous Message Tom Lane 2017-06-12 00:03:28 Re: Transactional sequence stuff breaks pg_upgrade