Hi! On Thu, Mar 31, 2016 at 4:59 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Tue, Mar 29, 2016 at 10:52 PM, Alexander Korotkov < > a.korot...@postgrespro.ru> wrote: > >> Hi, Andres! >> >> Please, find next revision of patch in attachment. >> >> > Couple of minor comments: > > + * The following two macroses > > is macroses right word to be used here? > > + * of this loop. It should be used as fullowing: > > /fullowing/following > > + * For local buffers usage of these macros shouldn't be used. > > isn't it better to write it as > > For local buffers, these macros shouldn't be used. > > > static int ts_ckpt_progress_comparator(Datum a, Datum b, void *arg); > > - > > Spurious line deletion. > All of above is fixed. + * Since buffers are pinned/unpinned very frequently, this functions tries > + * to pin buffer as cheap as possible. > > /this functions tries > > which functions are you referring here? Comment seems to be slightly > unclear. > I meant just PinBuffer() there. UnpinBuffer() has another comment in the body. Fixed. > ! if (XLogHintBitIsNeeded() && (pg_atomic_read_u32(&bufHdr->state) & > BM_PERMANENT)) > > Is there a reason that you have kept macro's to read refcount and > usagecount, but not for flags? > We could change dealing with flags to GET/SET macros. But I guess such change would make review more complicated, because it would touch some places which are unchanged for now. I think it could be done in a separate refactoring patch. Apart from this, I have verified that patch compiles on Windows and passed > regressions (make check)! > Thank you! I didn't manage to try this on Windows. > Nice work! > Thank you! ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
pinunpin-cas-8.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
