GenericXLogUnregister seems like a pretty horrid idea

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: GenericXLogUnregister seems like a pretty horrid idea
Date: 2016-04-09 18:15:44
Message-ID: 17980.1460225744@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I've been doing some code review in generic_xlog.c, and I find that
GenericXLogUnregister is implemented in what seems to me to be a
completely unsafe way: it shoves over the per-page array entries for
following pages. The problem with that is that it invalidates the Page
pointers previously returned for those pages by GenericXLogRegister;
an effect that is neither documented, nor reasonable for callers to
cope with.

I cannot see any very good reason why callers should need an unregister
function anyway: if they're doing something so complicated that they might
change their minds partway through an XLOG record through about whether a
page needs to be modified, that code is badly in need of redesign. (And
yes, I am asserting that against blinsert() in particular. That's not
nice code.)

I think we should get rid of this function. If people need to do
something like that, they should do GenericXLogAbort() and start over.

regards, tom lane

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2016-04-09 19:38:31 Re: Move PinBuffer and UnpinBuffer to atomics
Previous Message Tatsuo Ishii 2016-04-09 17:37:57 Re: multivariate statistics v14