Re: pgsql: Fix a couple of bugs in MultiXactId freezing

On 2013-12-03 13:14:38 -0500, Noah Misch wrote:
> On Tue, Dec 03, 2013 at 04:37:58PM +0100, Andres Freund wrote:
> > On 2013-12-03 10:29:54 -0500, Noah Misch wrote:
> > > Sorry, my original report was rather terse. I speak of the scenario exercised
> > > by the second permutation in that isolationtester spec. The multixact is
> > > later than VACUUM's cutoff_multi, so 9.3.1 does not freeze it at all. 9.3.2
> > > does freeze it to InvalidTransactionId per the code I cited in my first
> > > response on this thread, which wrongly removes a key lock.
> >
> > That one is clear, I was only confused about the Assert() you
> > reported. But I think I've explained that elsewhere.
> >
> > I currently don't see fixing the errorneous freezing of lockers (not the
> > updater though) without changing the wal format or synchronously waiting
> > for all lockers to end. Which both see like a no-go?
>
> Not fixing it at all is the real no-go. We'd take both of those undesirables
> before just tolerating the lost locks in 9.3.

I think it's changing the wal format then.

> The attached patch illustrates the approach I was describing earlier. It
> fixes the test case discussed above. I haven't verified that everything else
> in the system is ready for it, so this is just for illustration purposes.

That might be better than the current state because the potential damage
from such not frozen locks would be to get "could not access status of
transaction ..." errors (*).
But the problem I see with it is that a FOR UPDATE/NO KEY UPDATE lock taken out by
UPDATE is different than the respective lock taken out by SELECT FOR
SHARE:
typedef enum
{
MultiXactStatusForKeyShare = 0x00,
MultiXactStatusForShare = 0x01,
MultiXactStatusForNoKeyUpdate = 0x02,
MultiXactStatusForUpdate = 0x03,
/* an update that doesn't touch "key" columns */
MultiXactStatusNoKeyUpdate = 0x04,
/* other updates, and delete */
MultiXactStatusUpdate = 0x05
} MultiXactStatus;

Ignoring the difference that way isn't going to fly nicely.

*: which already are possible because we do not check multis properly
against OldestVisibleMXactId[] anymore.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services