+++ /dev/null
-From tgl@sss.pgh.pa.us Sun Aug 30 11:25:23 1998
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id LAA12607
- for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 11:25:20 -0400 (EDT)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id LAA15788;
- Sun, 30 Aug 1998 11:23:38 -0400 (EDT)
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-cc: dz@cs.unitn.it (Massimo Dal Zotto), hackers@postgreSQL.org
-Subject: Re: [HACKERS] flock patch breaks things here
-In-reply-to: Your message of Sun, 30 Aug 1998 08:19:52 -0400 (EDT)
- <199808301219.IAA08821@candle.pha.pa.us>
-Date: Sun, 30 Aug 1998 11:23:38 -0400
-Message-ID: <15786.904490618@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Status: RO
-
-Bruce Momjian <maillist@candle.pha.pa.us> writes:
-> Can't we just have configure check for flock(). Another idea is to
-> create a 'pid' file in the pgsql/data/base directory, and do a kill -0
-> to see if it is stil running before removing the lock.
-
-The latter approach is what I was going to suggest. Writing a pid file
-would be a fine idea anyway --- for one thing, it makes it a lot easier
-to write a "kill the postmaster" script. Given that the postmaster
-should write a pid file, a new postmaster should look for an existing
-pid file, and try to do a kill(pid, 0) on the number contained therein.
-If this doesn't return an error, then you figure there is already a
-postmaster running, complain, and exit. Otherwise you figure you is it,
-(re)write the pid file and away you go. Then pqcomm.c can just
-unconditionally delete any old file that's in the way of making the
-pipe.
-
-The pidfile checking and creation probably ought to go in postmaster.c,
-not down inside pqcomm.c. I never liked the fact that a critical
-interlock function was being done by a low-level library that one might
-not even want to invoke (if all your clients are using TCP, opening up
-the Unix-domain socket is a waste of time, no?).
-
-BTW, there is another problem with relying on flock on the socket file
-for this purpose: it opens up a hole for a denial-of-service attack.
-Anyone who can write the file can flock it. (We already had a problem
-with DOS via creating a dummy file at /tmp/.s.PGSQL.5432, but it would
-be harder to spot the culprit with an flock-based interference.)
-
- regards, tom lane
-
-From owner-pgsql-hackers@hub.org Sun Aug 30 12:27:41 1998
-Received: from hub.org (hub.org [209.47.148.200])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id MAA12976
- for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 12:27:37 -0400 (EDT)
-Received: from localhost (majordom@localhost) by hub.org (8.8.8/8.7.5) with SMTP id MAA09234; Sun, 30 Aug 1998 12:24:51 -0400 (EDT)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sun, 30 Aug 1998 12:23:26 +0000 (EDT)
-Received: (from majordom@localhost) by hub.org (8.8.8/8.7.5) id MAA09167 for pgsql-hackers-outgoing; Sun, 30 Aug 1998 12:23:25 -0400 (EDT)
-Received: from mambo.cs.unitn.it (mambo.cs.unitn.it [193.205.199.204]) by hub.org (8.8.8/8.7.5) with SMTP id MAA09150 for <hackers@postgreSQL.org>; Sun, 30 Aug 1998 12:23:08 -0400 (EDT)
-Received: from boogie.cs.unitn.it (dz@boogie [193.205.199.79]) by mambo.cs.unitn.it (8.6.12/8.6.12) with ESMTP id SAA29572; Sun, 30 Aug 1998 18:21:42 +0200
-Received: (from dz@localhost) by boogie.cs.unitn.it (8.8.5/8.6.9) id SAA05993; Sun, 30 Aug 1998 18:21:41 +0200
-From: Massimo Dal Zotto <dz@cs.unitn.it>
-Message-Id: <199808301621.SAA05993@boogie.cs.unitn.it>
-Subject: Re: [HACKERS] flock patch breaks things here
-To: hackers@postgreSQL.org (PostgreSQL Hackers)
-Date: Sun, 30 Aug 1998 18:21:41 +0200 (MET DST)
-Cc: tgl@sss.pgh.pa.us (Tom Lane)
-In-Reply-To: <15786.904490618@sss.pgh.pa.us> from "Tom Lane" at Aug 30, 98 11:23:38 am
-X-Mailer: ELM [version 2.4 PL24 ME4]
-MIME-Version: 1.0
-Content-Type: text/plain; charset=iso-8859-1
-Content-Transfer-Encoding: 8bit
-Sender: owner-pgsql-hackers@hub.org
-Precedence: bulk
-Status: ROr
-
->
-> Bruce Momjian <maillist@candle.pha.pa.us> writes:
-> > Can't we just have configure check for flock(). Another idea is to
-> > create a 'pid' file in the pgsql/data/base directory, and do a kill -0
-> > to see if it is stil running before removing the lock.
->
-> The latter approach is what I was going to suggest. Writing a pid file
-> would be a fine idea anyway --- for one thing, it makes it a lot easier
-> to write a "kill the postmaster" script. Given that the postmaster
-> should write a pid file, a new postmaster should look for an existing
-> pid file, and try to do a kill(pid, 0) on the number contained therein.
-> If this doesn't return an error, then you figure there is already a
-> postmaster running, complain, and exit. Otherwise you figure you is it,
-> (re)write the pid file and away you go. Then pqcomm.c can just
-> unconditionally delete any old file that's in the way of making the
-> pipe.
->
-> The pidfile checking and creation probably ought to go in postmaster.c,
-> not down inside pqcomm.c. I never liked the fact that a critical
-> interlock function was being done by a low-level library that one might
-> not even want to invoke (if all your clients are using TCP, opening up
-> the Unix-domain socket is a waste of time, no?).
->
-> BTW, there is another problem with relying on flock on the socket file
-> for this purpose: it opens up a hole for a denial-of-service attack.
-> Anyone who can write the file can flock it. (We already had a problem
-> with DOS via creating a dummy file at /tmp/.s.PGSQL.5432, but it would
-> be harder to spot the culprit with an flock-based interference.)
-
-This came to my mind, but I didn't think this would have happened so
-quickly. In my opinion the socket and the pidfile should be created in a
-directory owned by postgres, for example /tmp/.Pgsql-unix, like does X.
-
---
-Massimo Dal Zotto
-
-+----------------------------------------------------------------------+
-| Massimo Dal Zotto email: dz@cs.unitn.it |
-| Via Marconi, 141 phone: ++39-461-534251 |
-| 38057 Pergine Valsugana (TN) www: http://www.cs.unitn.it/~dz/ |
-| Italy pgp: finger dz@tango.cs.unitn.it |
-+----------------------------------------------------------------------+
-
-
-From owner-pgsql-hackers@hub.org Sun Aug 30 13:01:10 1998
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id NAA13785
- for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 13:01:09 -0400 (EDT)
-Received: from hub.org (hub.org [209.47.148.200]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id MAA29386 for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 12:58:24 -0400 (EDT)
-Received: from localhost (majordom@localhost) by hub.org (8.8.8/8.7.5) with SMTP id MAA11406; Sun, 30 Aug 1998 12:54:48 -0400 (EDT)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sun, 30 Aug 1998 12:52:22 +0000 (EDT)
-Received: (from majordom@localhost) by hub.org (8.8.8/8.7.5) id MAA11310 for pgsql-hackers-outgoing; Sun, 30 Aug 1998 12:52:20 -0400 (EDT)
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6]) by hub.org (8.8.8/8.7.5) with ESMTP id MAA11296 for <hackers@postgreSQL.org>; Sun, 30 Aug 1998 12:52:13 -0400 (EDT)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id MAA16094;
- Sun, 30 Aug 1998 12:50:55 -0400 (EDT)
-To: Massimo Dal Zotto <dz@cs.unitn.it>
-cc: hackers@postgreSQL.org (PostgreSQL Hackers)
-Subject: Re: [HACKERS] flock patch breaks things here
-In-reply-to: Your message of Sun, 30 Aug 1998 18:21:41 +0200 (MET DST)
- <199808301621.SAA05993@boogie.cs.unitn.it>
-Date: Sun, 30 Aug 1998 12:50:55 -0400
-Message-ID: <16092.904495855@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@hub.org
-Precedence: bulk
-Status: RO
-
-Massimo Dal Zotto <dz@cs.unitn.it> writes:
-> In my opinion the socket and the pidfile should be created in a
-> directory owned by postgres, for example /tmp/.Pgsql-unix, like does X.
-
-The pidfile belongs at the top level of the database directory (eg,
-/usr/local/pgsql/data/postmaster.pid), because what it actually
-represents is that there is a postmaster running *for that database
-group*.
-
-If you want to support multiple database sets on one machine (which I
-do), then the interlock has to be per database directory. Putting the
-pidfile into a common directory would mean we'd have to invent some
-kind of pidfile naming convention to keep multiple postmasters from
-tromping on each other. This is unnecessarily complex.
-
-I agree with you that putting the socket file into a less easily munged
-directory than /tmp would be a good idea for security. But that's a
-separate issue. On machines that understand stickybits for directories,
-the security hole is not really very big.
-
-At this point, the fact that /tmp/.s.PGSQL.port# is the socket path is
-effectively a version-independent aspect of the FE/BE protocol, and so
-we can't change it without breaking old applications. I'm not sure that
-that's worth the security improvement.
-
-What I'd like to see someday is a postmaster command line switch to tell
-it to use *only* TCP connections and not create a Unix socket at all.
-That hasn't been possible so far, because we were relying on the socket
-file to provide a safety interlock against starting multiple
-postmasters. But an interlock using a pidfile would be much better.
-(Look around; *every* other Unix daemon I know of that wants to ensure
-that there's only one of it uses a pidfile interlock. Not file locking.
-There's a reason why that's the well-trodden path.)
-
- regards, tom lane
-
-
-From owner-pgsql-hackers@hub.org Sun Aug 30 15:31:13 1998
-Received: from hub.org (hub.org [209.47.148.200])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id PAA15275
- for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 15:31:11 -0400 (EDT)
-Received: from localhost (majordom@localhost) by hub.org (8.8.8/8.7.5) with SMTP id PAA22194; Sun, 30 Aug 1998 15:27:20 -0400 (EDT)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sun, 30 Aug 1998 15:23:58 +0000 (EDT)
-Received: (from majordom@localhost) by hub.org (8.8.8/8.7.5) id PAA21800 for pgsql-hackers-outgoing; Sun, 30 Aug 1998 15:23:57 -0400 (EDT)
-Received: from thelab.hub.org (nat0118.mpoweredpc.net [142.177.188.118]) by hub.org (8.8.8/8.7.5) with ESMTP id PAA21696 for <hackers@postgreSQL.org>; Sun, 30 Aug 1998 15:22:51 -0400 (EDT)
-Received: from localhost (scrappy@localhost)
- by thelab.hub.org (8.9.1/8.8.8) with SMTP id QAA18542;
- Sun, 30 Aug 1998 16:21:29 -0300 (ADT)
- (envelope-from scrappy@hub.org)
-X-Authentication-Warning: thelab.hub.org: scrappy owned process doing -bs
-Date: Sun, 30 Aug 1998 16:21:28 -0300 (ADT)
-From: The Hermit Hacker <scrappy@hub.org>
-To: Tom Lane <tgl@sss.pgh.pa.us>
-cc: Massimo Dal Zotto <dz@cs.unitn.it>,
- PostgreSQL Hackers <hackers@postgreSQL.org>
-Subject: Re: [HACKERS] flock patch breaks things here
-In-Reply-To: <16092.904495855@sss.pgh.pa.us>
-Message-ID: <Pine.BSF.4.02.9808301618350.343-100000@thelab.hub.org>
-MIME-Version: 1.0
-Content-Type: TEXT/PLAIN; charset=US-ASCII
-Sender: owner-pgsql-hackers@hub.org
-Precedence: bulk
-Status: RO
-
-On Sun, 30 Aug 1998, Tom Lane wrote:
-
-> Massimo Dal Zotto <dz@cs.unitn.it> writes:
-> > In my opinion the socket and the pidfile should be created in a
-> > directory owned by postgres, for example /tmp/.Pgsql-unix, like does X.
->
-> The pidfile belongs at the top level of the database directory (eg,
-> /usr/local/pgsql/data/postmaster.pid), because what it actually
-> represents is that there is a postmaster running *for that database
-> group*.
-
- I have to agree with this one...but then it also negates the
-argument about the flock() DoS...*grin*
-
- BTW...I like the kill(pid,0) solution myself, primarily because it
-is, i think, the most portable solution.
-
- I would not consider a patch to remove the flock() solution and
-replace it with the kill(pid,0) solution a new feature, just an
-improvement of an existing one...either way, moving the pid file (or
-socket, for that matter) from /tmp should be listed as a security related
-requirement for v6.4 :)
-
-Marc G. Fournier
-Systems Administrator @ hub.org
-primary: scrappy@hub.org secondary: scrappy@{freebsd|postgresql}.org
-
-
-
-From owner-pgsql-hackers@hub.org Sun Aug 30 22:41:10 1998
-Received: from hub.org (hub.org [209.47.148.200])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id WAA01526
- for <maillist@candle.pha.pa.us>; Sun, 30 Aug 1998 22:41:08 -0400 (EDT)
-Received: from localhost (majordom@localhost) by hub.org (8.8.8/8.7.5) with SMTP id WAA29298; Sun, 30 Aug 1998 22:38:18 -0400 (EDT)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sun, 30 Aug 1998 22:35:05 +0000 (EDT)
-Received: (from majordom@localhost) by hub.org (8.8.8/8.7.5) id WAA29203 for pgsql-hackers-outgoing; Sun, 30 Aug 1998 22:35:03 -0400 (EDT)
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6]) by hub.org (8.8.8/8.7.5) with ESMTP id WAA29017 for <hackers@postgreSQL.org>; Sun, 30 Aug 1998 22:34:55 -0400 (EDT)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id WAA20075;
- Sun, 30 Aug 1998 22:34:41 -0400 (EDT)
-To: The Hermit Hacker <scrappy@hub.org>
-cc: PostgreSQL Hackers <hackers@postgreSQL.org>
-Subject: Re: [HACKERS] flock patch breaks things here
-In-reply-to: Your message of Sun, 30 Aug 1998 16:21:28 -0300 (ADT)
- <Pine.BSF.4.02.9808301618350.343-100000@thelab.hub.org>
-Date: Sun, 30 Aug 1998 22:34:40 -0400
-Message-ID: <20073.904530880@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@hub.org
-Precedence: bulk
-Status: ROr
-
-The Hermit Hacker <scrappy@hub.org> writes:
-> either way, moving the pid file (or
-> socket, for that matter) from /tmp should be listed as a security related
-> requirement for v6.4 :)
-
-Huh? There is no pid file being generated in /tmp (or anywhere else)
-at the moment. If we do add one, it should not go into /tmp for the
-reasons I gave before.
-
-Where the Unix-domain socket file lives is an entirely separate issue.
-
-If we move the socket out of /tmp then we have just kicked away all the
-work we did to preserve backwards compatibility of the FE/BE protocol
-with existing clients. Being able to talk to a 1.0 client isn't much
-good if you aren't listening where he's going to try to contact you.
-So I think I have to vote in favor of leaving the socket where it is.
-
- regards, tom lane
-
-
-From owner-pgsql-hackers@hub.org Mon Aug 31 11:31:19 1998
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.8.5/8.8.5) with ESMTP id LAA21195
- for <maillist@candle.pha.pa.us>; Mon, 31 Aug 1998 11:31:13 -0400 (EDT)
-Received: from hub.org (hub.org [209.47.148.200]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id LAA06827 for <maillist@candle.pha.pa.us>; Mon, 31 Aug 1998 11:17:41 -0400 (EDT)
-Received: from localhost (majordom@localhost) by hub.org (8.8.8/8.7.5) with SMTP id LAA24792; Mon, 31 Aug 1998 11:12:18 -0400 (EDT)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Mon, 31 Aug 1998 11:10:31 +0000 (EDT)
-Received: (from majordom@localhost) by hub.org (8.8.8/8.7.5) id LAA24742 for pgsql-hackers-outgoing; Mon, 31 Aug 1998 11:10:29 -0400 (EDT)
-Received: from trillium.nmsu.edu (trillium.NMSU.Edu [128.123.5.15]) by hub.org (8.8.8/8.7.5) with ESMTP id LAA24725 for <hackers@postgreSQL.org>; Mon, 31 Aug 1998 11:10:22 -0400 (EDT)
-Received: (from brook@localhost)
- by trillium.nmsu.edu (8.8.8/8.8.8) id JAA03282;
- Mon, 31 Aug 1998 09:09:01 -0600 (MDT)
-Date: Mon, 31 Aug 1998 09:09:01 -0600 (MDT)
-Message-Id: <199808311509.JAA03282@trillium.nmsu.edu>
-From: Brook Milligan <brook@trillium.NMSU.Edu>
-To: tgl@sss.pgh.pa.us
-CC: dg@informix.com, hackers@postgreSQL.org
-In-reply-to: <23042.904573041@sss.pgh.pa.us> (message from Tom Lane on Mon, 31
- Aug 1998 10:17:21 -0400)
-Subject: Re: [HACKERS] flock patch breaks things here
-References: <23042.904573041@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@hub.org
-Precedence: bulk
-Status: ROr
-
- I just came up with an idea that might help alleviate the /tmp security
- exposure without creating a backwards-compatibility problem. It works
- like this:
-
- 1. During installation, create a subdirectory of /tmp to hold Postgres'
- socket files and associated pid lockfiles. This subdirectory should be
- owned by the Postgres superuser and have permissions 755
- (world-readable, writable only by Postgres superuser). Maybe call it
- /tmp/.pgsql --- the name should start with a dot to keep it out of the
- way. (Bruce points out that some systems clear /tmp during reboot, so
- it might be that a postmaster will have to be prepared to recreate this
- directory at startup --- anyone know if subdirectories of /tmp are
- zapped too? My system doesn't do that...)
-
- ...
-
- I notice that on my system, the X11 socket files in /tmp/.X11-unix are
- actually symlinks to socket files in /usr/spool/sockets/X11. I dunno if
- it's worth our trouble to get into putting our sockets under /usr/spool
- or /var/spool or whatever --- seems like another configuration choice to
- mess up. It'd be nice if the socket directory lived somewhere where the
- parent dirs weren't world-writable, but this would mean one more thing
- that you have to have root permissions for in order to install pgsql.
-
-It seems like we need a directory for locks (= pid files) and one for
-sockets (perhaps the same one). I strongly suggest that the location
-for these be configurable. By default, it might make sense to put
-them in ~pgsql/locks and ~pgsql/sockets. It is easy (i.e., I'll be
-glad to do it) to modify configure.in to take options like
-
- --lock-dir=/var/spool/lock
- --socket-dir=/var/spool/sockets
-
-that set cc defines and have the code respond accordingly. This way,
-those who don't care (or don't have root access) can use the defaults,
-whereas those with root access who like to keep locks and sockets in a
-common place can do so easily. Either way, multiple postmasters (all
-compiled with the same options of course) can check the appropriate
-locks in the well-known places. Finally, drop the link into /tmp for
-the old socket and document that it will be disappearing at some
-point, and all is fine.
-
-If someone wants to give me some guidance on what preprocessor
-variables should be set in response to the above options (or something
-like them), I'll do the configure stuff.
-
-Cheers,
-Brook
-
-
+++ /dev/null
-From owner-pgsql-hackers@hub.org Thu Nov 26 08:31:13 1998
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id IAA24423
- for <maillist@candle.pha.pa.us>; Thu, 26 Nov 1998 08:31:08 -0500 (EST)
-Received: from hub.org (majordom@hub.org [209.47.148.200]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id IAA04554 for <maillist@candle.pha.pa.us>; Thu, 26 Nov 1998 08:04:30 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.1/8.9.1) with SMTP id HAA03761;
- Thu, 26 Nov 1998 07:56:37 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Thu, 26 Nov 1998 07:55:28 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.1/8.9.1) id HAA03689
- for pgsql-hackers-outgoing; Thu, 26 Nov 1998 07:55:26 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from orion.SAPserv.Hamburg.dsh.de (Tpolaris2.sapham.debis.de [53.2.131.8])
- by hub.org (8.9.1/8.9.1) with SMTP id HAA03674
- for <pgsql-hackers@postgreSQL.org>; Thu, 26 Nov 1998 07:55:19 -0500 (EST)
- (envelope-from wieck@sapserv.debis.de)
-Received: by orion.SAPserv.Hamburg.dsh.de
- for pgsql-hackers@postgreSQL.org
- id m0zj13G-000EBfC; Thu, 26 Nov 98 14:01 MET
-Message-Id: <m0zj13G-000EBfC@orion.SAPserv.Hamburg.dsh.de>
-From: jwieck@debis.com (Jan Wieck)
-Subject: Re: [HACKERS] Re: memory leak with Abort Transaction
-To: takehi-s@ascii.co.jp (SHIOZAKI Takehiko)
-Date: Thu, 26 Nov 1998 14:01:42 +0100 (MET)
-Cc: pgsql-hackers@postgreSQL.org
-Reply-To: jwieck@debis.com (Jan Wieck)
-In-Reply-To: <199811261240.VAA27516@libpc01.pb.ascii.co.jp> from "SHIOZAKI Takehiko" at Nov 26, 98 09:40:19 pm
-X-Mailer: ELM [version 2.4 PL25]
-Content-Type: text
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: RO
-
-SHIOZAKI Takehiko wrote:
-
->
-> Hello!
->
-> Releasing 6.4.1 is a good news.
-> But would you confirm the following "memory leak" problem?
-> It is reproducable on 6.4 (FreeBSD 2.2.7-RELEASE).
-
- It's an far too old problem. And as far as I remember, there
- are different locations in the code causing it.
-
- One place I remember well. It's in the tcop mainloop in
- PostgresMain(). The querytree list is malloc()'ed (there and
- in the parser) and free()'d after the query is processed -
- except the processing of the queries bails out with elog().
- In that case it never runs over the free() because the
- longjmp() kick's it back to the beginning of the loop.
-
-
-Jan
-
---
-
-#======================================================================#
-# It's easier to get forgiveness for being wrong than for being right. #
-# Let's break this rule - forgive me. #
-#======================================== jwieck@debis.com (Jan Wieck) #
-
-
-
-
-From owner-pgsql-hackers@hub.org Fri Mar 19 16:01:29 1999
-Received: from hub.org (majordom@hub.org [209.47.145.100])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id QAA05828
- for <maillist@candle.pha.pa.us>; Fri, 19 Mar 1999 16:01:22 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.2/8.9.1) with SMTP id PAA15701;
- Fri, 19 Mar 1999 15:59:51 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Fri, 19 Mar 1999 15:59:08 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.2/8.9.1) id PAA15551
- for pgsql-hackers-outgoing; Fri, 19 Mar 1999 15:59:05 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from andrew.cmu.edu (ANDREW.CMU.EDU [128.2.10.101])
- by hub.org (8.9.2/8.9.1) with ESMTP id PAA15524
- for <pgsql-hackers@postgresql.org>; Fri, 19 Mar 1999 15:58:53 -0500 (EST)
- (envelope-from er1p+@andrew.cmu.edu)
-Received: (from postman@localhost) by andrew.cmu.edu (8.8.5/8.8.2) id PAA29323 for pgsql-hackers@postgresql.org; Fri, 19 Mar 1999 15:58:50 -0500 (EST)
-Received: via switchmail; Fri, 19 Mar 1999 15:58:50 -0500 (EST)
-Received: from cloudy.me.cmu.edu via qmail
- ID </afs/andrew.cmu.edu/service/mailqs/q005/QF.cqwfdxK00gNtE0TVBp>;
- Fri, 19 Mar 1999 15:58:37 -0500 (EST)
-Received: from cloudy.me.cmu.edu via qmail
- ID </afs/andrew.cmu.edu/usr2/er1p/.Outgoing/QF.MqwfdrO00gNtEmTPh2>;
- Fri, 19 Mar 1999 15:58:31 -0500 (EST)
-Received: from mms.4.60.Jun.27.1996.03.05.56.sun4.41.EzMail.2.0.CUILIB.3.45.SNAP.NOT.LINKED.cloudy.me.cmu.edu.sun4m.412
- via MS.5.6.cloudy.me.cmu.edu.sun4_41;
- Fri, 19 Mar 1999 15:58:29 -0500 (EST)
-Message-ID: <wqwfdpu00gNtImTPUm@andrew.cmu.edu>
-Date: Fri, 19 Mar 1999 15:58:29 -0500 (EST)
-From: Erik Riedel <riedel+@CMU.EDU>
-To: pgsql-hackers@postgreSQL.org
-Subject: [HACKERS] aggregation memory leak and fix
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: ROr
-
-
-Platform: Alpha, Digital UNIX 4.0D
-Software: PostgreSQL 6.4.2 and 6.5 snaphot (11 March 1999)
-
-I have a table as follows:
-
-Table = lineitem
-+------------------------+----------------------------------+-------+
-| Field | Type | Length|
-+------------------------+----------------------------------+-------+
-| l_orderkey | int4 not null | 4 |
-| l_partkey | int4 not null | 4 |
-| l_suppkey | int4 not null | 4 |
-| l_linenumber | int4 not null | 4 |
-| l_quantity | float4 not null | 4 |
-| l_extendedprice | float4 not null | 4 |
-| l_discount | float4 not null | 4 |
-| l_tax | float4 not null | 4 |
-| l_returnflag | char() not null | 1 |
-| l_linestatus | char() not null | 1 |
-| l_shipdate | date | 4 |
-| l_commitdate | date | 4 |
-| l_receiptdate | date | 4 |
-| l_shipinstruct | char() not null | 25 |
-| l_shipmode | char() not null | 10 |
-| l_comment | char() not null | 44 |
-+------------------------+----------------------------------+-------+
-Index: lineitem_index_
-
-that ends up having on the order of 500,000 rows (about 100 MB on disk).
-
-I then run an aggregation query as:
-
---
--- Query 1
---
-select l_returnflag, l_linestatus, sum(l_quantity) as sum_qty,
-sum(l_extendedprice) as sum_base_price,
-sum(l_extendedprice*(1-l_discount)) as sum_disc_price,
-sum(l_extendedprice*(1-l_discount)*(1+l_tax)) as sum_charge,
-avg(l_quantity) as avg_qty, avg(l_extendedprice) as avg_price,
-avg(l_discount) as avg_disc, count(*) as count_order
-from lineitem
-where l_shipdate <= ('1998-12-01'::datetime - interval '90 day')::date
-group by l_returnflag, l_linestatus
-order by l_returnflag, l_linestatus;
-
-
-when I run this against 6.4.2, the postgres process grows to upwards of
-1 GB of memory (at which point something overflows and it dumps core) -
-I watch it grow through 200 MB, 400 MB, 800 MB, dies somewhere near 1 GB
-of allocated memory).
-
-If I take out a few of the "sum" expressions it gets better, removing
-sum_disk_price and sum_charge causes it to be only 600 MB and the query
-actually (eventually) completes. Takes about 10 minutes on my 500 MHz
-machine with 256 MB core and 4 GB of swap.
-
-The problem seems to be the memory allocation mechanism. Looking at a
-call trace, it is doing some kind of "sub query" plan for each row in
-the database. That means it does ExecEval and postquel_function and
-postquel_execute and all their friends for each row in the database.
-Allocating a couple hundred bytes for each one.
-
-The problem is that none of these allocations are freed - they seem to
-depend on the AllocSet to free them at the end of the transaction. This
-means it isn't a "true" leak, because the bytes are all freed at the
-(very) end of the transaction, but it does mean that the process grows
-to unreasonable size in the meantime. There is no need for this,
-because the individual expression results are aggregated as it goes
-along, so the intermediate nodes can be freed.
-
-I spent half a day last week chasing down the offending palloc() calls
-and execution stacks sufficiently that I think I found the right places
-to put pfree() calls.
-
-As a result, I have changes in the files:
-
-src/backend/executor/execUtils.c
-src/backend/executor/nodeResult.c
-src/backend/executor/nodeAgg.c
-src/backend/executor/execMain.c
-
-patches to these files are attached at the end of this message. These
-files are based on the 6.5.0 snapshot downloaded from ftp.postgreql.org
-on 11 March 1999.
-
-Apologies for sending patches to a non-released version. If anyone has
-problems applying the patches, I can send the full files (I wanted to
-avoid sending a 100K shell archive to the list). If anyone cares about
-reproducing my exact problem with the above table, I can provide the 100
-MB pg_dump file for download as well.
-
-Secondary Issue: the reason I did not use the 6.4.2 code to make my
-changes is because the AllocSet calls in that one were particularly
-egregious - they only had the skeleton of the allocsets code that exists
-in the 6.5 snapshots, so they were calling malloc() for all of the 8 and
-16 byte allocations that the above query causes.
-
-Using the fixed code reduces the maximum memory requirement on the above
-query to about 210 MB, and reduces the runtime to (an acceptable) 1.5
-minutes - a factor of more than 6x improvement on my 256 MB machine.
-
-Now the biggest part of the execution time is in the sort before the
-aggregation (which isn't strictly needed, but that is an optimization
-for another day).
-
-Open Issue: there is still a small "leak" that I couldn't eliminate, I
-think I chased it down to the constvalue allocated in
-execQual::ExecTargetList(), but I couldn't figure out where to properly
-free it. 8 bytes leaked was much better than 750 bytes, so I stopped
-banging my head on that particular item.
-
-Secondary Open Issue: what I did have to do to get down to 210 MB of
-core was reduce the minimum allocation size in AllocSet to 8 bytes from
-16 bytes. That reduces the 8 byte leak above to a true 8 byte, rather
-than a 16 byte leak. Otherwise, I think the size was 280 MB (still a
-big improvement on 1000+ MB). I only changed this in my code and I am
-not including a changed mcxt.c for that.
-
-I hope my changes are understandable/reasonable. Enjoy.
-
-Erik Riedel
-Carnegie Mellon University
-www.cs.cmu.edu/~riedel
-
---------------[aggregation_memory_patch.sh]-----------------------
-
-#! /bin/sh
-# This is a shell archive, meaning:
-# 1. Remove everything above the #! /bin/sh line.
-# 2. Save the resulting text in a file.
-# 3. Execute the file with /bin/sh (not csh) to create:
-# execMain.c.diff
-# execUtils.c.diff
-# nodeAgg.c.diff
-# nodeResult.c.diff
-# This archive created: Fri Mar 19 15:47:17 1999
-export PATH; PATH=/bin:/usr/bin:$PATH
-if test -f 'execMain.c.diff'
-then
- echo shar: "will not over-write existing file 'execMain.c.diff'"
-else
-cat << \SHAR_EOF > 'execMain.c.diff'
-583c
-
-.
-398a
-
-.
-396a
- /* XXX - clean up some more from ExecutorStart() - er1p */
- if (NULL == estate->es_snapshot) {
- /* nothing to free */
- } else {
- if (estate->es_snapshot->xcnt > 0) {
- pfree(estate->es_snapshot->xip);
- }
- pfree(estate->es_snapshot);
- }
-
- if (NULL == estate->es_param_exec_vals) {
- /* nothing to free */
- } else {
- pfree(estate->es_param_exec_vals);
- estate->es_param_exec_vals = NULL;
- }
-
-.
-SHAR_EOF
-fi
-if test -f 'execUtils.c.diff'
-then
- echo shar: "will not over-write existing file 'execUtils.c.diff'"
-else
-cat << \SHAR_EOF > 'execUtils.c.diff'
-368a
-}
-
-/* ----------------
- * ExecFreeExprContext
- * ----------------
- */
-void
-ExecFreeExprContext(CommonState *commonstate)
-{
- ExprContext *econtext;
-
- /* ----------------
- * get expression context. if NULL then this node has
- * none so we just return.
- * ----------------
- */
- econtext = commonstate->cs_ExprContext;
- if (econtext == NULL)
- return;
-
- /* ----------------
- * clean up memory used.
- * ----------------
- */
- pfree(econtext);
- commonstate->cs_ExprContext = NULL;
-}
-
-/* ----------------
- * ExecFreeTypeInfo
- * ----------------
- */
-void
-ExecFreeTypeInfo(CommonState *commonstate)
-{
- TupleDesc tupDesc;
-
- tupDesc = commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor;
- if (tupDesc == NULL)
- return;
-
- /* ----------------
- * clean up memory used.
- * ----------------
- */
- FreeTupleDesc(tupDesc);
- commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor = NULL;
-.
-274a
-
-.
-SHAR_EOF
-fi
-if test -f 'nodeAgg.c.diff'
-then
- echo shar: "will not over-write existing file 'nodeAgg.c.diff'"
-else
-cat << \SHAR_EOF > 'nodeAgg.c.diff'
-376a
- pfree(oldVal); /* XXX - new, let's free the old datum - er1p */
-.
-374a
- oldVal = value1[aggno]; /* XXX - save so we can free later - er1p */
-.
-112a
- Datum oldVal = (Datum) NULL; /* XXX - so that we can save and free on
-each iteration - er1p */
-.
-SHAR_EOF
-fi
-if test -f 'nodeResult.c.diff'
-then
- echo shar: "will not over-write existing file 'nodeResult.c.diff'"
-else
-cat << \SHAR_EOF > 'nodeResult.c.diff'
-278a
- pfree(resstate); node->resstate = NULL; /* XXX - new for us - er1p */
-.
-265a
- ExecFreeExprContext(&resstate->cstate); /* XXX - new for us - er1p */
- ExecFreeTypeInfo(&resstate->cstate); /* XXX - new for us - er1p */
-.
-SHAR_EOF
-fi
-exit 0
-# End of shell archive
-
-
-
-From er1p+@andrew.cmu.edu Fri Mar 19 19:43:27 1999
-Received: from po8.andrew.cmu.edu (PO8.ANDREW.CMU.EDU [128.2.10.108])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id TAA09183
- for <maillist@candle.pha.pa.us>; Fri, 19 Mar 1999 19:43:26 -0500 (EST)
-Received: (from postman@localhost) by po8.andrew.cmu.edu (8.8.5/8.8.2) id TAA11773; Fri, 19 Mar 1999 19:43:18 -0500 (EST)
-Received: via switchmail; Fri, 19 Mar 1999 19:43:18 -0500 (EST)
-Received: from cloudy.me.cmu.edu via qmail
- ID </afs/andrew.cmu.edu/service/mailqs/q007/QF.oqwiwLK00gNtQmTLgB>;
- Fri, 19 Mar 1999 19:43:05 -0500 (EST)
-Received: from mms.4.60.Jun.27.1996.03.05.56.sun4.41.EzMail.2.0.CUILIB.3.45.SNAP.NOT.LINKED.cloudy.me.cmu.edu.sun4m.412
- via MS.5.6.cloudy.me.cmu.edu.sun4_41;
- Fri, 19 Mar 1999 19:43:02 -0500 (EST)
-Message-ID: <YqwiwKW00gNt4mTKsv@andrew.cmu.edu>
-Date: Fri, 19 Mar 1999 19:43:02 -0500 (EST)
-From: Erik Riedel <riedel+@CMU.EDU>
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-Subject: Re: [HACKERS] aggregation memory leak and fix
-Cc: pgsql-hackers@postgreSQL.org
-In-Reply-To: <199903192223.RAA06691@candle.pha.pa.us>
-References: <199903192223.RAA06691@candle.pha.pa.us>
-Status: ROr
-
-
-> No apologies necessary. Glad to have someone digging into that area of
-> the code. We will gladly apply your patches to 6.5. However, I request
-> that you send context diffs(diff -c). Normal diffs are just too
-> error-prone in application. Send them, and I will apply them right
-> away.
->
-Context diffs attached. This was due to my ignorance of diff. When I
-made the other files, I though "hmm, these could be difficult to apply
-if the code has changed a bit, wouldn't it be good if they included a
-few lines before and after the fix". Now I know "-c".
-
-> Not sure why that is there? Perhaps for GROUP BY processing?
->
-Right, it is a result of the Group processing requiring sorted input.
-Just that it doesn't "require" sorted input, it "could" be a little more
-flexible and the sort wouldn't be necessary. Essentially this would be
-a single "AggSort" node that did the aggregation while sorting (probably
-with replacement selection rather than quicksort). This definitely
-would require some code/smarts that isn't there today.
-
-> > think I chased it down to the constvalue allocated in
-> > execQual::ExecTargetList(), but I couldn't figure out where to properly
-> > free it. 8 bytes leaked was much better than 750 bytes, so I stopped
-> > banging my head on that particular item.
->
-> Can you give me the exact line? Is it the palloc(1)?
->
-No, the 8 bytes seem to come from the ExecEvalExpr() call near line
-1530. Problem was when I tried to free these, I got "not in AllocSet"
-errors, so something more complicated was going on.
-
-Thanks.
-
-Erik
-
------------[aggregation_memory_patch.sh]----------------------
-
-#! /bin/sh
-# This is a shell archive, meaning:
-# 1. Remove everything above the #! /bin/sh line.
-# 2. Save the resulting text in a file.
-# 3. Execute the file with /bin/sh (not csh) to create:
-# execMain.c.diff
-# execUtils.c.diff
-# nodeAgg.c.diff
-# nodeResult.c.diff
-# This archive created: Fri Mar 19 19:35:42 1999
-export PATH; PATH=/bin:/usr/bin:$PATH
-if test -f 'execMain.c.diff'
-then
- echo shar: "will not over-write existing file 'execMain.c.diff'"
-else
-cat << \SHAR_EOF > 'execMain.c.diff'
-***
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
-execMain.c Thu Mar 11 23:59:11 1999
----
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
-execMain.c Fri Mar 19 15:03:28 1999
-***************
-*** 394,401 ****
---- 394,419 ----
-
- EndPlan(queryDesc->plantree, estate);
-
-+ /* XXX - clean up some more from ExecutorStart() - er1p */
-+ if (NULL == estate->es_snapshot) {
-+ /* nothing to free */
-+ } else {
-+ if (estate->es_snapshot->xcnt > 0) {
-+ pfree(estate->es_snapshot->xip);
-+ }
-+ pfree(estate->es_snapshot);
-+ }
-+
-+ if (NULL == estate->es_param_exec_vals) {
-+ /* nothing to free */
-+ } else {
-+ pfree(estate->es_param_exec_vals);
-+ estate->es_param_exec_vals = NULL;
-+ }
-+
- /* restore saved refcounts. */
- BufferRefCountRestore(estate->es_refcount);
-+
- }
-
- void
-***************
-*** 580,586 ****
- /*
- * initialize result relation stuff
- */
-!
- if (resultRelation != 0 && operation != CMD_SELECT)
- {
- /*
---- 598,604 ----
- /*
- * initialize result relation stuff
- */
-!
- if (resultRelation != 0 && operation != CMD_SELECT)
- {
- /*
-SHAR_EOF
-fi
-if test -f 'execUtils.c.diff'
-then
- echo shar: "will not over-write existing file 'execUtils.c.diff'"
-else
-cat << \SHAR_EOF > 'execUtils.c.diff'
-***
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
-execUtils.c Thu Mar 11 23:59:11 1999
----
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
-execUtils.c Fri Mar 19 14:55:59 1999
-***************
-*** 272,277 ****
---- 272,278 ----
- #endif
- i++;
- }
-+
- if (len > 0)
- {
- ExecAssignResultType(commonstate,
-***************
-*** 366,371 ****
---- 367,419 ----
-
- pfree(projInfo);
- commonstate->cs_ProjInfo = NULL;
-+ }
-+
-+ /* ----------------
-+ * ExecFreeExprContext
-+ * ----------------
-+ */
-+ void
-+ ExecFreeExprContext(CommonState *commonstate)
-+ {
-+ ExprContext *econtext;
-+
-+ /* ----------------
-+ * get expression context. if NULL then this node has
-+ * none so we just return.
-+ * ----------------
-+ */
-+ econtext = commonstate->cs_ExprContext;
-+ if (econtext == NULL)
-+ return;
-+
-+ /* ----------------
-+ * clean up memory used.
-+ * ----------------
-+ */
-+ pfree(econtext);
-+ commonstate->cs_ExprContext = NULL;
-+ }
-+
-+ /* ----------------
-+ * ExecFreeTypeInfo
-+ * ----------------
-+ */
-+ void
-+ ExecFreeTypeInfo(CommonState *commonstate)
-+ {
-+ TupleDesc tupDesc;
-+
-+ tupDesc = commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor;
-+ if (tupDesc == NULL)
-+ return;
-+
-+ /* ----------------
-+ * clean up memory used.
-+ * ----------------
-+ */
-+ FreeTupleDesc(tupDesc);
-+ commonstate->cs_ResultTupleSlot->ttc_tupleDescriptor = NULL;
- }
-
- /* ----------------------------------------------------------------
-SHAR_EOF
-fi
-if test -f 'nodeAgg.c.diff'
-then
- echo shar: "will not over-write existing file 'nodeAgg.c.diff'"
-else
-cat << \SHAR_EOF > 'nodeAgg.c.diff'
-***
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
-nodeAgg.c Thu Mar 11 23:59:11 1999
----
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
-nodeAgg.c Fri Mar 19 15:01:21 1999
-***************
-*** 110,115 ****
---- 110,116 ----
- isNull2 = FALSE;
- bool qual_result;
-
-+ Datum oldVal = (Datum) NULL; /* XXX - so that we can save and free
-on each iteration - er1p */
-
- /* ---------------------
- * get state info from node
-***************
-*** 372,379 ****
---- 373,382 ----
- */
- args[0] = value1[aggno];
- args[1] = newVal;
-+ oldVal = value1[aggno]; /* XXX - save so we can free later - er1p */
- value1[aggno] = (Datum) fmgr_c(&aggfns->xfn1,
- (FmgrValues *) args, &isNull1);
-+ pfree(oldVal); /* XXX - new, let's free the old datum - er1p */
- Assert(!isNull1);
- }
- }
-SHAR_EOF
-fi
-if test -f 'nodeResult.c.diff'
-then
- echo shar: "will not over-write existing file 'nodeResult.c.diff'"
-else
-cat << \SHAR_EOF > 'nodeResult.c.diff'
-***
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/611/src/backend/executor/
-nodeResult.c Thu Mar 11 23:59:12 1999
----
-/afs/ece.cmu.edu/project/lcs/lcs-004/er1p/postgres/612/src/backend/executor/
-nodeResult.c Fri Mar 19 14:57:26 1999
-***************
-*** 263,268 ****
---- 263,270 ----
- * is freed at end-transaction time. -cim 6/2/91
- * ----------------
- */
-+ ExecFreeExprContext(&resstate->cstate); /* XXX - new for us - er1p */
-+ ExecFreeTypeInfo(&resstate->cstate); /* XXX - new for us - er1p */
- ExecFreeProjectionInfo(&resstate->cstate);
-
- /* ----------------
-***************
-*** 276,281 ****
---- 278,284 ----
- * ----------------
- */
- ExecClearTuple(resstate->cstate.cs_ResultTupleSlot);
-+ pfree(resstate); node->resstate = NULL; /* XXX - new for us - er1p */
- }
-
- void
-SHAR_EOF
-fi
-exit 0
-# End of shell archive
-
-
-From owner-pgsql-hackers@hub.org Fri Mar 19 21:01:15 1999
-Received: from hub.org (majordom@hub.org [209.47.145.100])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id VAA11368
- for <maillist@candle.pha.pa.us>; Fri, 19 Mar 1999 21:01:13 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.2/8.9.1) with SMTP id UAA40887;
- Fri, 19 Mar 1999 20:59:47 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Fri, 19 Mar 1999 20:58:14 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.2/8.9.1) id UAA40637
- for pgsql-hackers-outgoing; Fri, 19 Mar 1999 20:58:12 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from candle.pha.pa.us (maillist@s5-03.ppp.op.net [209.152.195.67])
- by hub.org (8.9.2/8.9.1) with ESMTP id UAA40620
- for <pgsql-hackers@postgreSQL.org>; Fri, 19 Mar 1999 20:58:02 -0500 (EST)
- (envelope-from maillist@candle.pha.pa.us)
-Received: (from maillist@localhost)
- by candle.pha.pa.us (8.9.0/8.9.0) id UAA11263;
- Fri, 19 Mar 1999 20:58:00 -0500 (EST)
-From: Bruce Momjian <maillist@candle.pha.pa.us>
-Message-Id: <199903200158.UAA11263@candle.pha.pa.us>
-Subject: Re: [HACKERS] aggregation memory leak and fix
-In-Reply-To: <YqwiwKW00gNt4mTKsv@andrew.cmu.edu> from Erik Riedel at "Mar 19, 1999 7:43: 2 pm"
-To: riedel+@CMU.EDU (Erik Riedel)
-Date: Fri, 19 Mar 1999 20:58:00 -0500 (EST)
-Cc: pgsql-hackers@postgreSQL.org
-X-Mailer: ELM [version 2.4ME+ PL47 (25)]
-MIME-Version: 1.0
-Content-Type: text/plain; charset=US-ASCII
-Content-Transfer-Encoding: 7bit
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: ROr
-
->
-> > No apologies necessary. Glad to have someone digging into that area of
-> > the code. We will gladly apply your patches to 6.5. However, I request
-> > that you send context diffs(diff -c). Normal diffs are just too
-> > error-prone in application. Send them, and I will apply them right
-> > away.
-> >
-> Context diffs attached. This was due to my ignorance of diff. When I
-> made the other files, I though "hmm, these could be difficult to apply
-> if the code has changed a bit, wouldn't it be good if they included a
-> few lines before and after the fix". Now I know "-c".
-
-Applied.
-
-> > Not sure why that is there? Perhaps for GROUP BY processing?
-> >
-> Right, it is a result of the Group processing requiring sorted input.
-> Just that it doesn't "require" sorted input, it "could" be a little more
-> flexible and the sort wouldn't be necessary. Essentially this would be
-> a single "AggSort" node that did the aggregation while sorting (probably
-> with replacement selection rather than quicksort). This definitely
-> would require some code/smarts that isn't there today.
-
-I think you will find make_groupPlan adds the sort as needed by the
-GROUP BY. I assume you are suggesting to do the aggregate/GROUP on unsorted
-data, which is hard to do in a flexible way.
-
-> > > think I chased it down to the constvalue allocated in
-> > > execQual::ExecTargetList(), but I couldn't figure out where to properly
-> > > free it. 8 bytes leaked was much better than 750 bytes, so I stopped
-> > > banging my head on that particular item.
-> >
-> > Can you give me the exact line? Is it the palloc(1)?
-> >
-> No, the 8 bytes seem to come from the ExecEvalExpr() call near line
-> 1530. Problem was when I tried to free these, I got "not in AllocSet"
-> errors, so something more complicated was going on.
-
-Yes, if you look inside ExecEvalExpr(), you will see it tries to get a
-value for the expression(Datum). It may return an int, float4, or a
-string. In the last case, that is actually a pointer and not a specific
-value.
-
-So, in some cases, the value can just be thrown away, or it may be a
-pointer to memory that can be freed after the call to heap_formtuple()
-later in the function. The trick is to find the function call in
-ExecEvalExpr() that is allocating something, and conditionally free
-values[] after the call to heap_formtuple(). If you don't want find it,
-perhaps you can send me enough info so I can see it here.
-
-I wonder whether it is the call to CreateTupleDescCopy() inside
-ExecEvalVar()?
-
-Another problem I just fixed is that fjIsNull was not being pfree'ed if
-it was used with >64 targets, but I don't think that affects you.
-
-I also assume you have run your recent patch through the the
-test/regression tests, so see it does not cause some other area to fail,
-right?
-
---
- Bruce Momjian | http://www.op.net/~candle
- maillist@candle.pha.pa.us | (610) 853-3000
- + If your life is a hard drive, | 830 Blythe Avenue
- + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
-
-
-From owner-pgsql-hackers@hub.org Sat Mar 20 12:01:44 1999
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id MAA24855
- for <maillist@candle.pha.pa.us>; Sat, 20 Mar 1999 12:01:43 -0500 (EST)
-Received: from hub.org (majordom@hub.org [209.47.145.100]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id LAA11985 for <maillist@candle.pha.pa.us>; Sat, 20 Mar 1999 11:58:48 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.2/8.9.1) with SMTP id LAA12367;
- Sat, 20 Mar 1999 11:57:17 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sat, 20 Mar 1999 11:55:22 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.2/8.9.1) id LAA12026
- for pgsql-hackers-outgoing; Sat, 20 Mar 1999 11:55:17 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6])
- by hub.org (8.9.2/8.9.1) with ESMTP id LAA11871
- for <pgsql-hackers@postgreSQL.org>; Sat, 20 Mar 1999 11:54:57 -0500 (EST)
- (envelope-from tgl@sss.pgh.pa.us)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id LAA28068;
- Sat, 20 Mar 1999 11:48:58 -0500 (EST)
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-cc: riedel+@CMU.EDU (Erik Riedel), pgsql-hackers@postgreSQL.org
-Subject: Re: [HACKERS] aggregation memory leak and fix
-In-reply-to: Your message of Fri, 19 Mar 1999 21:33:33 -0500 (EST)
- <199903200233.VAA11816@candle.pha.pa.us>
-Date: Sat, 20 Mar 1999 11:48:58 -0500
-Message-ID: <28066.921948538@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: ROr
-
-Bruce Momjian <maillist@candle.pha.pa.us> writes:
-> My only quick solution would seem to be to add a new "expression" memory
-> context, that can be cleared after every tuple is processed, clearing
-> out temporary values allocated inside an expression.
-
-Right, this whole problem of growing backend memory use during a large
-SELECT (or COPY, or probably a few other things) is one of the things
-that we were talking about addressing by revising the memory management
-structure.
-
-I think what we want inside the executor is a distinction between
-storage that must live to the end of the statement and storage that is
-only needed while processing the current tuple. The second kind of
-storage would go into a separate context that gets flushed every so
-often. (It could be every tuple, or every dozen or hundred tuples
-depending on what seems the best tradeoff of cycles against memory
-usage.)
-
-I'm not sure that just two contexts is enough, either. For example in
- SELECT field1, SUM(field2) GROUP BY field1;
-the working memory for the SUM aggregate could not be released after
-each tuple, but perhaps we don't want it to live for the whole statement
-either --- in that case we'd need a per-group context. (This particular
-example isn't very convincing, because the same storage for the SUM
-*could* be recycled from group to group. But I don't know whether it
-actually *is* reused or not. If fresh storage is palloc'd for each
-instantiation of SUM then we have a per-group leak in this scenario.
-In any case, I'm not sure all aggregate functions have constant memory
-requirements that would let them recycle storage across groups.)
-
-What we need to do is work out what the best set of memory context
-definitions is, and then decide on a strategy for making sure that
-lower-level routines allocate their return values in the right context.
-It'd be nice if the lower-level routines could still call palloc() and
-not have to worry about this explicitly --- otherwise we'll break not
-only a lot of our own code but perhaps a lot of user code. (User-
-specific data types and SPI code all use palloc, no?)
-
-I think it is too late to try to fix this for 6.5, but it ought to be a
-top priority for 6.6.
-
- regards, tom lane
-
-
-From tgl@sss.pgh.pa.us Sun Mar 21 16:01:46 1999
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id QAA00139
- for <maillist@candle.pha.pa.us>; Sun, 21 Mar 1999 16:01:45 -0500 (EST)
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id PAA27737 for <maillist@candle.pha.pa.us>; Sun, 21 Mar 1999 15:52:38 -0500 (EST)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id PAA14946;
- Sun, 21 Mar 1999 15:50:20 -0500 (EST)
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-cc: pgsql-hackers@postgreSQL.org
-Subject: Re: [HACKERS] aggregation memory leak and fix
-In-reply-to: Your message of Sun, 21 Mar 1999 14:20:39 -0500 (EST)
- <199903211920.OAA28744@candle.pha.pa.us>
-Date: Sun, 21 Mar 1999 15:50:20 -0500
-Message-ID: <14944.922049420@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Status: ROr
-
-Bruce Momjian <maillist@candle.pha.pa.us> writes:
->> What we need to do is work out what the best set of memory context
->> definitions is, and then decide on a strategy for making sure that
->> lower-level routines allocate their return values in the right context.
-
-> Let's suppose that we want to free all the memory used as expression
-> intermediate values after each row is processed.
-> It is my understanding that all these are created in utils/adt/*.c
-> files, and that the entry point to all those functions via
-> fmgr()/fmgr_c().
-
-That's probably the bulk of the specific calls of palloc(). Someone
-(Jan?) did a scan of the code a while ago looking for palloc() calls,
-and there aren't that many outside of the data-type-specific functions.
-But we'd have to look individually at all the ones that are elsewhere.
-
-> So, if we go into an expression memory context before calling
-> fmgr/fmgr_c in the executor, and return to the normal context after the
-> function call, all our intermediates are trapped in the expression
-> memory context.
-
-OK, so you're saying we leave the data-type-specific functions as is
-(calling palloc() to allocate their result areas), and make each call
-site specifically responsible for setting the context that palloc() will
-allocate from? That could work, I think. We'd need to see what side
-effects it'd have on other uses of palloc().
-
-What we'd probably want is to use a stack discipline for the current
-palloc-target memory context: when you set the context, you get back the
-ID of the old context, and you are supposed to restore that old context
-before returning.
-
-> At the end of each row, we just free the expression memory context. In
-> almost all cases, the data is stored in tuples, and we can free it. In
-> a few cases like aggregates, we have to save off the value we need to
-> keep before freeing the expression context.
-
-Actually, nodeAgg would just have to set an appropriate context before
-calling fmgr to execute the aggregate's transition functions, and then
-it wouldn't need an extra copy step. The results would come back in the
-right context already.
-
-> In fact, you could even optimize the cleanup to only do free'ing if
-> some expression memory was allocated. In most cases, it is not.
-
-Jan's stuff should already fall through pretty quickly if there's
-nothing in the context, I think. Note that what we want to do between
-tuples is a "context clear" of the expression context, not a "context
-delete" and then "context create" a new expression context. Context
-clear should be a pretty quick no-op if nothing's been allocated in that
-context...
-
-> In fact the nodeAgg.c patch that I backed out attempted to do that,
-> though because there wasn't code that checked if the Datum was
-> pg_type.typbyval, it didn't work 100%.
-
-Right. But if we approach it this way (clear the context at appropriate
-times) rather than thinking in terms of explicitly pfree'ing individual
-objects, life gets much simpler. Also, if we insist on being able to
-pfree individual objects inside a context, we can't use Jan's faster
-allocator! Remember, the reason it is faster and lower overhead is that
-it doesn't keep track of individual objects, only pools.
-
-I'd like to see us head in the direction of removing most of the
-explicit pfree calls that exist now, and instead rely on clearing
-memory contexts at appropriate times in order to manage memory.
-The fewer places where we need pfree, the more contexts can be run
-with the low-overhead space allocator. Also, the fewer explicit
-pfrees we need, the simpler and more reliable the code gets.
-
- regards, tom lane
-
-From owner-pgsql-hackers@hub.org Sun Mar 21 16:01:49 1999
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id QAA00149
- for <maillist@candle.pha.pa.us>; Sun, 21 Mar 1999 16:01:48 -0500 (EST)
-Received: from hub.org (majordom@hub.org [209.47.145.100]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id PAA27950 for <maillist@candle.pha.pa.us>; Sun, 21 Mar 1999 15:56:07 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.2/8.9.1) with SMTP id PAA39413;
- Sun, 21 Mar 1999 15:54:51 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sun, 21 Mar 1999 15:54:31 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.2/8.9.1) id PAA39249
- for pgsql-hackers-outgoing; Sun, 21 Mar 1999 15:54:27 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6])
- by hub.org (8.9.2/8.9.1) with ESMTP id PAA39235
- for <pgsql-hackers@postgreSQL.org>; Sun, 21 Mar 1999 15:54:21 -0500 (EST)
- (envelope-from tgl@sss.pgh.pa.us)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id PAA14946;
- Sun, 21 Mar 1999 15:50:20 -0500 (EST)
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-cc: pgsql-hackers@postgreSQL.org
-Subject: Re: [HACKERS] aggregation memory leak and fix
-In-reply-to: Your message of Sun, 21 Mar 1999 14:20:39 -0500 (EST)
- <199903211920.OAA28744@candle.pha.pa.us>
-Date: Sun, 21 Mar 1999 15:50:20 -0500
-Message-ID: <14944.922049420@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: RO
-
-Bruce Momjian <maillist@candle.pha.pa.us> writes:
->> What we need to do is work out what the best set of memory context
->> definitions is, and then decide on a strategy for making sure that
->> lower-level routines allocate their return values in the right context.
-
-> Let's suppose that we want to free all the memory used as expression
-> intermediate values after each row is processed.
-> It is my understanding that all these are created in utils/adt/*.c
-> files, and that the entry point to all those functions via
-> fmgr()/fmgr_c().
-
-That's probably the bulk of the specific calls of palloc(). Someone
-(Jan?) did a scan of the code a while ago looking for palloc() calls,
-and there aren't that many outside of the data-type-specific functions.
-But we'd have to look individually at all the ones that are elsewhere.
-
-> So, if we go into an expression memory context before calling
-> fmgr/fmgr_c in the executor, and return to the normal context after the
-> function call, all our intermediates are trapped in the expression
-> memory context.
-
-OK, so you're saying we leave the data-type-specific functions as is
-(calling palloc() to allocate their result areas), and make each call
-site specifically responsible for setting the context that palloc() will
-allocate from? That could work, I think. We'd need to see what side
-effects it'd have on other uses of palloc().
-
-What we'd probably want is to use a stack discipline for the current
-palloc-target memory context: when you set the context, you get back the
-ID of the old context, and you are supposed to restore that old context
-before returning.
-
-> At the end of each row, we just free the expression memory context. In
-> almost all cases, the data is stored in tuples, and we can free it. In
-> a few cases like aggregates, we have to save off the value we need to
-> keep before freeing the expression context.
-
-Actually, nodeAgg would just have to set an appropriate context before
-calling fmgr to execute the aggregate's transition functions, and then
-it wouldn't need an extra copy step. The results would come back in the
-right context already.
-
-> In fact, you could even optimize the cleanup to only do free'ing if
-> some expression memory was allocated. In most cases, it is not.
-
-Jan's stuff should already fall through pretty quickly if there's
-nothing in the context, I think. Note that what we want to do between
-tuples is a "context clear" of the expression context, not a "context
-delete" and then "context create" a new expression context. Context
-clear should be a pretty quick no-op if nothing's been allocated in that
-context...
-
-> In fact the nodeAgg.c patch that I backed out attempted to do that,
-> though because there wasn't code that checked if the Datum was
-> pg_type.typbyval, it didn't work 100%.
-
-Right. But if we approach it this way (clear the context at appropriate
-times) rather than thinking in terms of explicitly pfree'ing individual
-objects, life gets much simpler. Also, if we insist on being able to
-pfree individual objects inside a context, we can't use Jan's faster
-allocator! Remember, the reason it is faster and lower overhead is that
-it doesn't keep track of individual objects, only pools.
-
-I'd like to see us head in the direction of removing most of the
-explicit pfree calls that exist now, and instead rely on clearing
-memory contexts at appropriate times in order to manage memory.
-The fewer places where we need pfree, the more contexts can be run
-with the low-overhead space allocator. Also, the fewer explicit
-pfrees we need, the simpler and more reliable the code gets.
-
- regards, tom lane
-
-
-From owner-pgsql-hackers@hub.org Wed Mar 24 19:10:53 1999
-Received: from renoir.op.net (root@renoir.op.net [209.152.193.4])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id TAA00906
- for <maillist@candle.pha.pa.us>; Wed, 24 Mar 1999 19:10:52 -0500 (EST)
-Received: from hub.org (majordom@hub.org [209.47.145.100]) by renoir.op.net (o1/$ Revision: 1.18 $) with ESMTP id NAA24258 for <maillist@candle.pha.pa.us>; Wed, 24 Mar 1999 13:09:47 -0500 (EST)
-Received: from localhost (majordom@localhost)
- by hub.org (8.9.2/8.9.1) with SMTP id NAA60743;
- Wed, 24 Mar 1999 13:07:26 -0500 (EST)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Wed, 24 Mar 1999 13:06:47 +0000 (EST)
-Received: (from majordom@localhost)
- by hub.org (8.9.2/8.9.1) id NAA60556
- for pgsql-hackers-outgoing; Wed, 24 Mar 1999 13:06:43 -0500 (EST)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-Received: from po7.andrew.cmu.edu (PO7.ANDREW.CMU.EDU [128.2.10.107])
- by hub.org (8.9.2/8.9.1) with ESMTP id NAA60540
- for <pgsql-hackers@postgreSQL.org>; Wed, 24 Mar 1999 13:06:25 -0500 (EST)
- (envelope-from er1p+@andrew.cmu.edu)
-Received: (from postman@localhost) by po7.andrew.cmu.edu (8.8.5/8.8.2) id NAA06323; Wed, 24 Mar 1999 13:06:16 -0500 (EST)
-Received: via switchmail; Wed, 24 Mar 1999 13:06:16 -0500 (EST)
-Received: from cloudy.me.cmu.edu via qmail
- ID </afs/andrew.cmu.edu/service/mailqs/q001/QF.cqyGa::00gNtI0TZtD>;
- Wed, 24 Mar 1999 13:06:02 -0500 (EST)
-Received: from cloudy.me.cmu.edu via qmail
- ID </afs/andrew.cmu.edu/usr2/er1p/.Outgoing/QF.sqyGa8G00gNtImTGBe>;
- Wed, 24 Mar 1999 13:06:00 -0500 (EST)
-Received: from mms.4.60.Jun.27.1996.03.05.56.sun4.41.EzMail.2.0.CUILIB.3.45.SNAP.NOT.LINKED.cloudy.me.cmu.edu.sun4m.412
- via MS.5.6.cloudy.me.cmu.edu.sun4_41;
- Wed, 24 Mar 1999 13:05:58 -0500 (EST)
-Message-ID: <QqyGa6600gNtMmTG1o@andrew.cmu.edu>
-Date: Wed, 24 Mar 1999 13:05:58 -0500 (EST)
-From: Erik Riedel <riedel+@CMU.EDU>
-To: Bruce Momjian <maillist@candle.pha.pa.us>
-Subject: Re: [HACKERS] aggregation memory leak and fix
-Cc: pgsql-hackers@postgreSQL.org
-In-Reply-To: <199903240611.BAA01206@candle.pha.pa.us>
-References: <199903240611.BAA01206@candle.pha.pa.us>
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: RO
-
-
-> I am interested to see if it fixes the expression leak you saw. I have
-> not committed this yet. I want to look at it some more.
->
-I'm afraid that this doesn't seem to have any effect on my query.
-
-Looking at your code, I think the problem is that most of the
-allocations in my query are on the top part of the if statement that
-you modified (i.e. the == SQLlanguageId part). Below is a snippet of
-a trace from my query, with approximate line numbers for execQual.c
-with your patch applied:
-
-(execQual) language == SQLlanguageId (execQual.c:757)
-(execQual) execute postquel_function (execQual.c:759)
-(mcxt) MemoryContextAlloc 32 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 16 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 528 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 56 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 88 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 24 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 65 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 48 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(execQual) exit qual context (execQual.c:862)
-(mcxt) MemoryContextAlloc 60 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 528 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes
-(execQual) return from postquel_function (execQual.c:764)
-(execQual) return from ExecEvalFuncArgs (execQual.c:792)
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(execQual) exit qual context (execQual.c:862)
-(mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 128 bytes
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(mcxt) MemoryContextAlloc 8 bytes in <Qual manager>-heap
-(execQual) exit qual context (execQual.c:862)
-
-<pattern repeats>
-
-(execQual) language == SQLlanguageId (execQual.c:757)
-(execQual) execute postquel_function (execQual.c:759)
-(mcxt) MemoryContextAlloc 32 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 16 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 528 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 56 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 88 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 24 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 65 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 48 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 8 bytes in ** Blank Portal **-heap
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(execQual) exit qual context (execQual.c:862)
-(mcxt) MemoryContextAlloc 60 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 64 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 528 bytes
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 16 bytes
-(execQual) return from postquel_function (execQual.c:764)
-(execQual) return from ExecEvalFuncArgs (execQual.c:792)
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(execQual) exit qual context (execQual.c:862)
-(mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextAlloc 108 bytes in ** Blank Portal **-heap
-(mcxt) MemoryContextFree in ** Blank Portal **-heap freed 128 bytes
-(execQual) else clause NOT SQLlanguageId (execQual.c:822)
-(execQual) install qual memory context (execQual.c:858)
-(mcxt) MemoryContextAlloc 8 bytes in <Qual manager>-heap
-(execQual) exit qual context (execQual.c:862)
-
-
-the MemoryContext lines give the name of the portal where each
-allocation is happening - you see that your Qual manager only captures
-a very small number (one) of the allocations, the rest are in the
-upper part of the if statement.
-
-Note that I also placed a printf next to your EndPortalAllocMode() and
-StartPortalAllocMode() fix in ExecQual() - I believe this is what is
-supposed to clear the portal and free the memory - and that printf
-never appears in the above trace.
-
-Sorry if the trace is a little confusing, but I hope that it helps you
-zero in.
-
-Erik
-
-
-
-
-
-
-
-From owner-pgsql-hackers@hub.org Sat May 15 23:13:50 1999
-Received: from hub.org (hub.org [209.167.229.1])
- by candle.pha.pa.us (8.9.0/8.9.0) with ESMTP id XAA29144
- for <maillist@candle.pha.pa.us>; Sat, 15 May 1999 23:13:49 -0400 (EDT)
-Received: from hub.org (hub.org [209.167.229.1])
- by hub.org (8.9.3/8.9.3) with ESMTP id XAA25173;
- Sat, 15 May 1999 23:11:03 -0400 (EDT)
- (envelope-from owner-pgsql-hackers@hub.org)
-Received: by hub.org (TLB v0.10a (1.23 tibbs 1997/01/09 00:29:32)); Sat, 15 May 1999 23:10:29 +0000 (EDT)
-Received: (from majordom@localhost)
- by hub.org (8.9.3/8.9.3) id XAA25111
- for pgsql-hackers-outgoing; Sat, 15 May 1999 23:10:27 -0400 (EDT)
- (envelope-from owner-pgsql-hackers@postgreSQL.org)
-X-Authentication-Warning: hub.org: majordom set sender to owner-pgsql-hackers@postgreSQL.org using -f
-Received: from sss.sss.pgh.pa.us (sss.pgh.pa.us [206.210.65.6])
- by hub.org (8.9.3/8.9.3) with ESMTP id XAA25092
- for <pgsql-hackers@postgreSQL.org>; Sat, 15 May 1999 23:10:22 -0400 (EDT)
- (envelope-from tgl@sss.pgh.pa.us)
-Received: from sss.sss.pgh.pa.us (localhost [127.0.0.1])
- by sss.sss.pgh.pa.us (8.9.1/8.9.1) with ESMTP id XAA17752
- for <pgsql-hackers@postgreSQL.org>; Sat, 15 May 1999 23:09:46 -0400 (EDT)
-To: pgsql-hackers@postgreSQL.org
-Subject: [HACKERS] Memory leaks in relcache
-Date: Sat, 15 May 1999 23:09:46 -0400
-Message-ID: <17750.926824186@sss.pgh.pa.us>
-From: Tom Lane <tgl@sss.pgh.pa.us>
-Sender: owner-pgsql-hackers@postgreSQL.org
-Precedence: bulk
-Status: ROr
-
-I have been looking into why a reference to a nonexistent table, eg
- INSERT INTO nosuchtable VALUES(1);
-leaks a small amount of memory per occurrence. What I find is a
-memory leak in the indexscan support. Specifically,
-RelationGetIndexScan in backend/access/index/genam.c palloc's both
-an IndexScanDesc and some keydata storage. The IndexScanDesc
-block is eventually pfree'd, at the bottom of CatalogIndexFetchTuple
-in backend/catalog/indexing.c. But the keydata block is not.
-
-This wouldn't matter so much if the palloc were coming from a
-transaction-local context. But what we're doing is a lookup in pg_class
-on behalf of RelationBuildDesc in backend/utils/cache/relcache.c, and
-it's done a MemoryContextSwitchTo into the global CacheCxt before
-starting the lookup. Therefore, the un-pfreed block represents a
-permanent memory leak.
-
-In fact, *every* reference to a relation that is not already present in
-the relcache causes a similar leak. The error case is just the one that
-is easiest to repeat. The missing pfree of the keydata block is
-probably causing a bunch of other short-term and long-term leaks too.
-
-It seems to me there are two things to fix here: indexscan ought to
-pfree everything it pallocs, and RelationBuildDesc ought to be warier
-about how much work gets done with CacheCxt as the active palloc
-context. (Even if indexscan didn't leak anything ordinarily, there's
-still the risk of elog(ERROR) causing an abort before the indexscan code
-gets to clean up.)
-
-Comments? In particular, where is the cleanest place to add the pfree
-of the keydata block? I don't especially like the fact that callers
-of index_endscan have to clean up the toplevel scan block; I think that
-ought to happen inside index_endscan.
-
- regards, tom lane
-
-
projects / postgresql / commitdiff