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

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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: [COMMITTERS] pgsql: Support ALTER TABLESPACE name SET/RESET ( tablespace_options ).
Date: 2010-01-06 23:11:39
Message-ID: 28402.1262819499@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-committerspgsql-hackers
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.

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 ;-)

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).

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

			regards, tom lane

In response to

Responses

pgsql-hackers by date

Next:From: Tim BunceDate: 2010-01-06 23:14:05
Subject: Re: Status of plperl inter-sp calling
Previous:From: Kevin GrittnerDate: 2010-01-06 23:11:18
Subject: Re: Testing with concurrent sessions

pgsql-committers by date

Next:From: Bruce MomjianDate: 2010-01-06 23:23:51
Subject: pgsql: PG_MAJORVERSION: For simplicity, use PG_MAJORVERSION rather than
Previous:From: Tom LaneDate: 2010-01-06 23:00:03
Subject: pgsql: Fix spccache.c to not suppose that a cache entry will live across

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