Re: [pgsql-patches] Phantom Command IDs, updated patch

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
Cc: pgsql-patches(at)postgresql(dot)org
Subject: Re: [pgsql-patches] Phantom Command IDs, updated patch
Date: 2007-02-08 16:25:40
Message-ID: 200702081625.l18GPeQ09861@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Heikki Linnakangas wrote:
> Here's an updated version of the phantom command ids patch.
>
> I found one more subtle safety issue. The array and hash table for
> phantom command ids is dynamically grown when HeapTupleHeaderSetCmax is
> called. Unfortunately, since HeapTupleHeaderSetCmax is used inside a
> critical sections, running out of memory while trying to grow them would
> cause a PANIC. That's why I moved the SetXmax/SetCmax calls outside
> critical sections in heapam.c. I believe that's safe; if a backend
> aborts after setting the xmax/cmax, no-one is going to care about the
> xid of an aborted transaction in there.
>
> Per Tom's suggestion, I replaced the function cache code in fmgr.c and
> similar code in plperl.c, pltcl.c, plpgsql/pl_comp.c and plpython.c to
> use xmin+tid instead of xmin+cmin for the up-to-dateness check. I don't
> have any tcl, perl or python test cases handy to test them, but the
> change is small and essentially same for all of the above. Is there any
> regression tests for the PL languages?
>
> I made cmin and cmax system attributes aliases for the same physical
> commandid field. I support the idea of a complete overhaul of those
> system attributes, but let's do that in a separate patch.
>
> To measure the overhead, I ran a plpgsql test case that updates a single
> row 10000 times in a loop, generating a new phantom command id in each
> iteration. The test took ~5% longer with the patch, so I think that's
> acceptable. I couldn't measure a difference with pgbench (as expected).
>
> I think the patch is ready. Please remove the PHANTOMCID_DEBUG define
> and ifdef blocks before applying.

Heikki, I found something odd in your patch. You had an extra
parentheses at the end of the line in the orginal and new version of the
patch (attached). I removed it before applying, but I just wanted to
confirm this was OK.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachment Content-Type Size
unknown_filename text/plain 658 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2007-02-08 16:27:45 Re: [pgsql-patches] Phantom Command IDs, updated patch
Previous Message Bruce Momjian 2007-02-08 16:22:18 Re: [pgsql-patches] Phantom Command IDs, updated patch

Browse pgsql-patches by date

  From Date Subject
Next Message Alvaro Herrera 2007-02-08 16:27:45 Re: [pgsql-patches] Phantom Command IDs, updated patch
Previous Message Bruce Momjian 2007-02-08 16:22:18 Re: [pgsql-patches] Phantom Command IDs, updated patch