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

Re: pgstattuple locking fix

From: "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com>
To: <pgsql-patches(at)postgresql(dot)org>
Cc: "ITAGAKI Takahiro" <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>,"Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: pgstattuple locking fix
Date: 2007-10-22 10:07:53
Message-ID: 471C7679.2000705@enterprisedb.com (view raw or flat)
Thread:
Lists: pgsql-patches
Heikki Linnakangas wrote:
> ITAGAKI Takahiro wrote:
>> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>>> ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
>>>> Here is a trivial fix of locking issue in pgstattuple().
>>> Hmm, is this really a bug, and if so how far back does it go?
>>> I'm thinking that having a pin on the buffer should be enough to
>>> call PageGetHeapFreeSpace.
>> Hmm... we might use pd_upper and pd_lower at different times,
>> but the error is ok for pgstattuple purpose.
>> (It might be dangerous for tuple insertion, though.)
>> Inconsistent usage of locks is confusable -- remove them, please.
> 
> No I think the original patch was right. You can indeed read
> inconsistent values for pd_upper and pd_lower, though the window is very
> small. But a bigger issue is that in 8.3 PageGetHeapFreeSpace counts the
> used line pointers to see if MaxHeapTuplesPerPage has been reached, and
> I'm not convinced you can do that safely without holding a lock.

On second thought, we do call PageGetHeapFreeSpace without holding a
lock in heap_page_prune_opt as well, so it better be safe. Looking
closer at PageGetHeapFreeSpace, I think it is. The
return value can be bogus, of course.

That's worth noting in the comments:

*** src/backend/storage/page/bufpage.c  21 Sep 2007 21:25:42 -0000      1.75
--- src/backend/storage/page/bufpage.c  22 Oct 2007 10:06:02 -0000
***************
*** 506,511 ****
--- 506,514 ----
   * or dead line pointers it'd be possible to have too many line pointers.
   * To avoid breaking code that assumes MaxHeapTuplesPerPage is a hard
limit
   * on the number of line pointers, we make this extra check.)
+  *
+  * You don't need to hold a lock on the page to call this function,
but if
+  * you don't, the return value should be considered a hint only.
   */
  Size
  PageGetHeapFreeSpace(Page page)

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

In response to

Responses

pgsql-patches by date

Next:From: Pavel StehuleDate: 2007-10-22 10:48:08
Subject: EXECUTE USING for plpgsql (for 8.4)
Previous:From: Heikki LinnakangasDate: 2007-10-22 09:54:10
Subject: Re: pgstattuple locking fix

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