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

From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: 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-07-22 15:01:59
Message-ID: CAFcNs+o_20mSQRjgQB7+rRU6W0wYPPYQjJQfdENT2HHnSznsew@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 17, 2014 at 8:02 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:
>
> > That means should I "FlushRelationBuffers(rel)" before change the
> > relpersistence?
>
> I don't think that'd help. I think what this means that you simply
> cannot change the relpersistence of the old relation before the heap
> swap is successful. So I guess it has to be something like (pseudocode):
>
> OIDNewHeap = make_new_heap(..);
> newrel = heap_open(OIDNewHeap, AEL);
>
> /*
> * Change the temporary relation to be unlogged/logged. We have to do
> * that here so buffers for the new relfilenode will have the right
> * persistency set while the original filenode's buffers won't get read
> * in with the wrong (i.e. new) persistency setting. Otherwise a
> * rollback after the rewrite would possibly result with buffers for the
> * original filenode having the wrong persistency setting.
> *
> * NB: This relies on swap_relation_files() also swapping the
> * persistency. That wouldn't work for pg_class, but that can't be
> * unlogged anyway.
> */
> AlterTableChangeCatalogToLoggedOrUnlogged(newrel);
> FlushRelationBuffers(newrel);
> /* copy heap data into newrel */
> finish_heap_swap();
>
> And then in swap_relation_files() also copy the persistency.
>
>
> That's the best I can come up right now at least.
>

Isn't better if we can set the relpersistence as an argument to
"make_new_heap" ?

I'm thinking to change the make_new_heap:

From:

make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp,
LOCKMODE lockmode)

To:

make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence,
LOCKMODE lockmode)

That way we can create the new heap with the appropriate relpersistence, so
in the swap_relation_files also copy the persistency, of course.

And after copy the heap data to the new table (ATRewriteTable) change
relpersistence of the OldHeap's indexes, because in the "finish_heap_swap"
they'll be rebuild.

Thoughts?

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2014-07-22 15:08:03 pgsql: Diagnose incompatible OpenLDAP versions during build and test.
Previous Message Andrew Dunstan 2014-07-22 14:55:17 Re: Some bogus results from prairiedog