Skip site navigation (1) Skip section navigation (2)

Re: [COMMITTERS] pgsql: Support ALTER TABLESPACE name SET/RESET ( tablespace_options ).

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Support ALTER TABLESPACE name SET/RESET ( tablespace_options ).
Date: 2010-01-07 00:50:48
Message-ID: 603c8f071001061650l6fa0595ct5ef12a43f3b3de75@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-committerspgsql-hackers
On Wed, Jan 6, 2010 at 6:11 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> With this patch, a CLOBBER_CACHE_ALWAYS build starts falling apart all
>> over the place :-(.  Looks like you blew the memory management somehow;
>> it appears to be using a previously pfree'd pointer:
>
> Actually, there were three different problems there:
>
> 1. Relying on a HASH_REMOVE'd hashtable entry to still be present and
> valid.  This is at least bad style.  I wonder if we should tweak the
> dynahash code to memset a free'd entry to 7F's like pfree does.

Oops.

> 2. Assuming that a cache entry will remain in existence across a catalog
> access.  This will work except when it doesn't, ie, when a cache flush
> occurs during the table access.  You've just learned the hard way what
> CLOBBER_CACHE_ALWAYS testing is good for ;-)

Yeah.  I tried that at an earlier stage of development, but obviously
I should have retested with the final version.

> 3. Invoking tablespace_reloptions while switched into
> CacheMemoryContext.  This isn't a crasher, but it strikes me as a bad
> idea because if reloptions.c happens to leak anything, that'll represent
> a permanent (session-lifespan) memory leak.  And it's complicated enough
> that being sure it doesn't leak anything is hard.  I think you should
> invoke tablespace_reloptions in the caller's memory context, and then if
> it succeeds, copy the result into CacheMemoryContext.  That would
> probably require fixing the problem you noted earlier today about
> making TableSpaceOpts be a valid bytea value, so that it'll be easy to
> copy (then you can use datumCopy, for instance).

Hmm.  That seems a little crufty, but it makes sense.  Will do.

> I fixed the first two because they were in the way of investigating
> another problem, but #3 still needs your attention.

Thanks, and sorry for the hassle.

...Robert

In response to

pgsql-hackers by date

Next:From: Robert HaasDate: 2010-01-07 00:52:02
Subject: Re: Serializable Isolation without blocking
Previous:From: Greg StarkDate: 2010-01-07 00:45:04
Subject: Re: unresolved bugs

pgsql-committers by date

Next:From: Tom LaneDate: 2010-01-07 01:41:12
Subject: pgsql: Make configure check the version of Perl we're building with, and
Previous:From: Tom LaneDate: 2010-01-07 00:25:18
Subject: pgsql: Alter the configure script to fail immediately if the C compiler

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group