On Tue, Apr 26, 2016 at 11:47 AM, Andres Freund <andres@anarazel.de> wrote:
> Thanks for looking into this.
>
> On 2016-04-26 11:43:06 +0900, Michael Paquier wrote:
>> On Sun, Apr 24, 2016 at 11:51 AM, Andres Freund <andres@anarazel.de> wrote:
>> > 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.
>
> You mean the bit about making smgr invalidations logged?
Sorry for the lack of precision in my words. I was referring to your
idea of replacing CacheInvalidateSmgr() by CacheInvalidateRelcache()
sounds like a good option as this would make the invalidation to be
logged at commit. Thinking about the logging of smgr invalidations,
this is quite interesting. But what would we actually gain in doing
that? Do you foresee any advantages in doing so? The only case where
this would be useful now is for vm_extend by looking at the code.
>> As the invalidation patch is aimed at being backpatched, this may be
>> something to do as well in back-branches.
>
> I'm a bit split here. I think forcing processing of invalidations at
> moments they've previously never been processed is a bit risky for the
> back branches. But on the other hand relcache invalidations are only
> processed at end-of-xact, which isn't really correct for the code at
> hand :/
Oh, OK. So you mean that this patch is not aimed for back-branches
with this new record type, but that's only for HEAD.. To be honest, I
thought that we had better address this issue on back-branches with a
patch close to what you are proposing (which is more or less what
Simon has actually sent), and keep the change of CacheInvalidateSmgr()
in visibilitymap.c to be HEAD-only.
--
Michael
