Heikki Linnakangas wrote: > ITAGAKI Takahiro wrote: >> Tom Lane <[EMAIL PROTECTED]> wrote: >> >>> ITAGAKI Takahiro <[EMAIL PROTECTED]> 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 ---------------------------(end of broadcast)--------------------------- TIP 4: Have you searched our list archives? http://archives.postgresql.org
