Re: [GSoC2014] Patch ALTER TABLE ... SET LOGGED

From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, Thom Brown <thom(at)linux(dot)com>, Christoph Berg <cb(at)df7cb(dot)de>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>
Subject: Re: [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Date: 2014-08-22 20:01:25
Message-ID: CAFcNs+oOZN7DoUeJ7ZkWjVKHf4zOu6W1QqR1s9X4RPsD22btAQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 22, 2014 at 4:45 PM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:
>
> > 2. The patch seems to think that it can sometimes be safe to change
> > the relpersistence of an existing relation. Unless you can be sure
> > that no buffers can possibly be present in shared_buffers and nobody
> > will use an existing relcache entry to read a new one in, it's not,
> > because the buffers won't have the right BM_PERSISTENT marking. I'm
> > very nervous about the fact that this patch seems not to have touched
> > bufmgr.c, but maybe I'm missing something.
>
> Right. Andres pointed this out previously, and the patch was updated
> consequently. The only remaining case in which we do that, again AFAIR,
> is for indexes, just after the table has been rewritten and just before
> the indexes are reindexed. There should be no buffer access of the old
> indexes at that point, so there would be no buffer marked with the wrong
> flag. Also, the table is locked access-exclusively (is that a word?),
> so no other transaction could possibly be reading buffers for its
> indexes.
>
> I pointed out, in the email just before pushing the patch, that perhaps
> we should pass down the new relpersistence flag into finish_heap_swap,
> and from there we could pass it down to reindex_index() which is where
> it would be needed. I'm not sure it's worth the trouble, but I think we
> can still ask Fabrizio to rework that part.
>
> Maybe it is me missing something.
>
> BTW why is it that index_build() checks the heap's relpersistence flag
> rather than the index'?
>

I can rework this part if it's a real concern.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-08-22 20:23:31 Re: [GSoC2014] Patch ALTER TABLE ... SET LOGGED
Previous Message Alvaro Herrera 2014-08-22 19:47:22 Re: [GSoC2014] Patch ALTER TABLE ... SET LOGGED