Re: Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.
Date: 2016-04-26 02:43:06
Message-ID: CAB7nPqQv02D+z_A8Ou4ycy5KYCkk2NLCAdrNivAmghhhmpnT+g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Apr 24, 2016 at 11:51 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> Here's a patch doing so. Note that, after putting the record into RM_XACT_ID
> first, I've decided to make it a RM_STANDBY_ID type record. I think it's
> likely that this is going to be needed beyond transaction commits, and
> it generally seems to fit better into standby.c; even if it's a bit
> awkward that commit records contain similar data. To avoid duplicating
> the *desc.c code, I've exposed standby_desc_invalidations() to also be
> used by xactdesc.c.

I have been eyeballing this patch for some time since yesterday and
did some tests, and that's really neat. Having the invalidation stuff
in standby.c makes quite some sense as well.

+ * wanting to emit more WAL recoreds).
s/recoreds/records/

> The reason this all ends up working nonetheless is that the
> heap_inplace_update()s in vacuum triggers a CacheInvalidateHeapTuple()
> which queues a relcache invalidation, which in turn does a
> RelationCloseSmgr() in RelationClearRelation(). Thereby effectively
> doing a transactional CacheInvalidateSmgr(). But that seems more than
> fragile.

You have spent quite some time on that. As things stand that's indeed
a bit fragile..

> ISTM we should additionally replace the CacheInvalidateSmgr() with a
> CacheInvalidateRelcache() and document that that implies an smgr
> invalidation. Alternatively we could log smgr (and relmapper)
> invalidations as well, but that's not quite non-invasive either; but
> might be a good long-term idea to keep things simpler.
>
> Comments?

Yeah, this looks like a good idea at the end. As the invalidation
patch is aimed at being backpatched, this may be something to do as
well in back-branches.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-04-26 02:47:19 Re: Suspicious behaviour on applying XLOG_HEAP2_VISIBLE.
Previous Message Tom Lane 2016-04-26 02:40:52 Re: Can we improve this error message?