pgsql: Include the backend ID in the relpath of temporary relations.

Lists: pgsql-committerspgsql-hackers
From: rhaas(at)postgresql(dot)org (Robert Haas)
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-13 20:10:54
Message-ID: 20100813201054.19F6C7541D7@cvs.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Log Message:
-----------
Include the backend ID in the relpath of temporary relations.

This allows us to reliably remove all leftover temporary relation
files on cluster startup without reference to system catalogs or WAL;
therefore, we no longer include temporary relations in XLOG_XACT_COMMIT
and XLOG_XACT_ABORT WAL records.

Since these changes require including a backend ID in each
SharedInvalSmgrMsg, the size of the SharedInvalidationMessage.id
field has been reduced from two bytes to one, and the maximum number
of connections has been reduced from INT_MAX / 4 to 2^23-1. It would
be possible to remove these restrictions by increasing the size of
SharedInvalidationMessage by 4 bytes, but right now that doesn't seem
like a good trade-off.

Review by Jaime Casanova and Tom Lane.

Modified Files:
--------------
pgsql/doc/src/sgml:
storage.sgml (r1.32 -> r1.33)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/storage.sgml?r1=1.32&r2=1.33)
pgsql/src/backend/access/heap:
visibilitymap.c (r1.10 -> r1.11)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/heap/visibilitymap.c?r1=1.10&r2=1.11)
pgsql/src/backend/access/nbtree:
nbtsort.c (r1.125 -> r1.126)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/nbtree/nbtsort.c?r1=1.125&r2=1.126)
pgsql/src/backend/access/transam:
twophase.c (r1.62 -> r1.63)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/twophase.c?r1=1.62&r2=1.63)
xact.c (r1.297 -> r1.298)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xact.c?r1=1.297&r2=1.298)
xlogutils.c (r1.71 -> r1.72)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xlogutils.c?r1=1.71&r2=1.72)
pgsql/src/backend/catalog:
catalog.c (r1.90 -> r1.91)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/catalog.c?r1=1.90&r2=1.91)
heap.c (r1.374 -> r1.375)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/heap.c?r1=1.374&r2=1.375)
index.c (r1.337 -> r1.338)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/index.c?r1=1.337&r2=1.338)
namespace.c (r1.128 -> r1.129)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/namespace.c?r1=1.128&r2=1.129)
storage.c (r1.10 -> r1.11)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/storage.c?r1=1.10&r2=1.11)
toasting.c (r1.33 -> r1.34)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/catalog/toasting.c?r1=1.33&r2=1.34)
pgsql/src/backend/commands:
copy.c (r1.328 -> r1.329)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/copy.c?r1=1.328&r2=1.329)
sequence.c (r1.169 -> r1.170)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/sequence.c?r1=1.169&r2=1.170)
tablecmds.c (r1.339 -> r1.340)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/tablecmds.c?r1=1.339&r2=1.340)
pgsql/src/backend/postmaster:
bgwriter.c (r1.68 -> r1.69)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/postmaster/bgwriter.c?r1=1.68&r2=1.69)
pgsql/src/backend/storage/buffer:
bufmgr.c (r1.256 -> r1.257)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/buffer/bufmgr.c?r1=1.256&r2=1.257)
localbuf.c (r1.89 -> r1.90)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/buffer/localbuf.c?r1=1.89&r2=1.90)
pgsql/src/backend/storage/file:
fd.c (r1.157 -> r1.158)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/file/fd.c?r1=1.157&r2=1.158)
pgsql/src/backend/storage/freespace:
freespace.c (r1.77 -> r1.78)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/freespace/freespace.c?r1=1.77&r2=1.78)
pgsql/src/backend/storage/smgr:
md.c (r1.151 -> r1.152)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/smgr/md.c?r1=1.151&r2=1.152)
smgr.c (r1.121 -> r1.122)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/smgr/smgr.c?r1=1.121&r2=1.122)
pgsql/src/backend/utils:
probes.d (r1.12 -> r1.13)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/probes.d?r1=1.12&r2=1.13)
pgsql/src/backend/utils/adt:
dbsize.c (r1.32 -> r1.33)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/adt/dbsize.c?r1=1.32&r2=1.33)
pgsql/src/backend/utils/cache:
inval.c (r1.98 -> r1.99)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/cache/inval.c?r1=1.98&r2=1.99)
relcache.c (r1.311 -> r1.312)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/cache/relcache.c?r1=1.311&r2=1.312)
pgsql/src/backend/utils/misc:
guc.c (r1.566 -> r1.567)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/guc.c?r1=1.566&r2=1.567)
pgsql/src/include/access:
xlog_internal.h (r1.33 -> r1.34)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/access/xlog_internal.h?r1=1.33&r2=1.34)
pgsql/src/include/catalog:
catalog.h (r1.49 -> r1.50)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/catalog/catalog.h?r1=1.49&r2=1.50)
storage.h (r1.5 -> r1.6)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/catalog/storage.h?r1=1.5&r2=1.6)
pgsql/src/include/postmaster:
bgwriter.h (r1.15 -> r1.16)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/postmaster/bgwriter.h?r1=1.15&r2=1.16)
pgsql/src/include/storage:
bufmgr.h (r1.124 -> r1.125)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/bufmgr.h?r1=1.124&r2=1.125)
relfilenode.h (r1.25 -> r1.26)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/relfilenode.h?r1=1.25&r2=1.26)
sinval.h (r1.59 -> r1.60)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/sinval.h?r1=1.59&r2=1.60)
smgr.h (r1.71 -> r1.72)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/smgr.h?r1=1.71&r2=1.72)
pgsql/src/include/utils:
inval.h (r1.49 -> r1.50)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/inval.h?r1=1.49&r2=1.50)
rel.h (r1.124 -> r1.125)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/rel.h?r1=1.124&r2=1.125)


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-13 21:51:08
Message-ID: 22664.1281736268@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

rhaas(at)postgresql(dot)org (Robert Haas) writes:
> Include the backend ID in the relpath of temporary relations.

A couple of the buildfarm members don't like this patch. I think you
missed making some edits in some dtrace calls.

regards, tom lane


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-13 22:38:29
Message-ID: AANLkTi=T4j890L67wkUf7Vhd4bb28yyBv4vBpzFh3UhF@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

On Fri, Aug 13, 2010 at 5:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> rhaas(at)postgresql(dot)org (Robert Haas) writes:
>> Include the backend ID in the relpath of temporary relations.
>
> A couple of the buildfarm members don't like this patch.  I think you
> missed making some edits in some dtrace calls.

Well, I guess that's why we have a buildfarm. Working on it now...

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


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: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-13 22:58:04
Message-ID: AANLkTikUreb=K8LvtLh3gvJTDi8DDXCSAENOd3PRocMv@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

On Fri, Aug 13, 2010 at 6:38 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Aug 13, 2010 at 5:51 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> rhaas(at)postgresql(dot)org (Robert Haas) writes:
>>> Include the backend ID in the relpath of temporary relations.
>>
>> A couple of the buildfarm members don't like this patch.  I think you
>> missed making some edits in some dtrace calls.
>
> Well, I guess that's why we have a buildfarm.  Working on it now...

I have taken a crack at fixing this but someone who understands DTrace
better than I do may want to check and see if the changes look sane.
It appears to me that we have no documentation - not even so much as a
source code comment - explaining how these probes are supposed to work
or what the arguments to each one are intended mean. That may not be
ideal.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company


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: Re: [COMMITTERS] pgsql: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-13 23:23:14
Message-ID: 24817.1281741794@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I have taken a crack at fixing this but someone who understands DTrace
> better than I do may want to check and see if the changes look sane.
> It appears to me that we have no documentation - not even so much as a
> source code comment - explaining how these probes are supposed to work
> or what the arguments to each one are intended mean.

http://developer.postgresql.org/pgdocs/postgres/dynamic-trace.html#DTRACE-PROBE-POINT-TABLE

(... which you now need to update ...)

I think your confusion may stem from the fact that the definition of the
buffer-read-done probe was actually wrong, AFAICS. The docs say its
last 3 args were bools, which was reasonable, but the definition said
int for the first of those. Which is what you want now ...

regards, tom lane


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: Re: [COMMITTERS] pgsql: Include the backend ID in the relpath of temporary relations.
Date: 2010-08-14 02:19:58
Message-ID: AANLkTimN7Ej1Fiyr1E3p+iJHbp6Mvq-ue9z71-1DJ7Qg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

On Fri, Aug 13, 2010 at 7:23 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I have taken a crack at fixing this but someone who understands DTrace
>> better than I do may want to check and see if the changes look sane.
>> It appears to me that we have no documentation - not even so much as a
>> source code comment - explaining how these probes are supposed to work
>> or what the arguments to each one are intended mean.
>
> http://developer.postgresql.org/pgdocs/postgres/dynamic-trace.html#DTRACE-PROBE-POINT-TABLE
>
> (... which you now need to update ...)
>
> I think your confusion may stem from the fact that the definition of the
> buffer-read-done probe was actually wrong, AFAICS.  The docs say its
> last 3 args were bools, which was reasonable, but the definition said
> int for the first of those.  Which is what you want now ...

No, it was the original patch that mangled that. I think the real
problem is that (1) I didn't test with dtrace enabled when writing the
patch, or maybe I did somewhere in the middle but not at the end and
(2) I didn't realize there were docs.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company