Re: pg_prewarm(some more observations in patch)

From: Amit kapila <amit(dot)kapila(at)huawei(dot)com>
To: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: "cedric(at)2ndquadrant(dot)com" <cedric(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Stefan Keller <sfkeller(at)gmail(dot)com>
Subject: Re: pg_prewarm(some more observations in patch)
Date: 2012-07-08 05:32:25
Message-ID: 6C0B27F7206C9E4CA54AE035729E9C382851CA08@szxeml509-mbs
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

From: pgsql-hackers-owner(at)postgresql(dot)org [pgsql-hackers-owner(at)postgresql(dot)org] on behalf of Jeff Janes [jeff(dot)janes(at)gmail(dot)com]
>For the implementation:

>I think that for most users, making them know or care about forks and
> block numbers is too much. It would be nice if there were a
> single-argument form: pg_prewarm(relation) which loaded all of either
> main, or all of all forks, using 'buffer'. This seems like a good
> default. Also, the last two arguments are NULL in all the given
> examples. Do we expect those to be used only for experimental
> purposes by hackers, or are those of general interest?
I agree with you.
2 forms of the function can exist one with only one argument and other
with the arguments as specified in you interface. This can solve the purpose of all kind of users.
In the first form there should be defaults for all other values.

1. For the prewarm type(prefetch,read,buffer), it would have been better if either it is enum or
an case insensitive string. It could have been more convienient from user perspective.

+ if (PG_ARGISNULL(4))
+ last_block = nblocks - 1;
+ else
+ {
+ last_block = PG_GETARG_INT64(4);
+ if (last_block > nblocks)
+ ereport(ERROR,
+ errmsg("ending block number " INT64_FORMAT " exceeds number of blocks in relation " INT64_FORMAT, last_block, nblocks)));
+ }

It can add additional check if last block number is less than first block number, then report meaningful error.
As for this kind of case, it will not load any buffers and returns successfully.

3. + else if (ptype == PREWARM_READ)
+ {
+ /*
+ * In read mode, we actually read the blocks, but not into shared
+ * buffers. This is more portable than prefetch mode (it works
+ * everywhere) and is synchronous.
+ */
+ RelationOpenSmgr(rel);

Is it required to call RelationOpenSmgr(rel) as in the begining already it is done?

With Regards,
Amit Kapila.

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2012-07-08 07:13:09 expression evaluation with expected datatypes
Previous Message Satoshi Nagayasu 2012-07-08 04:17:52 Re: New statistics for WAL buffer dirty writes