Remove sort files

The only unusual part is the use of system("find...rm...") to
remove the files in pg_sorttemp. Seemed cleaner than doing fancy
directory walking code in C.

Should this figure out and use a path to the find command? Unless
postgres sets CWD, is it possible as-is to coerce it into rming some
files you did not intend to have rm'd? I didn't have/spend time
looking at it with a fine toothcomb, but usually calling find and rm
without an explicit path is a Bad Thing(tm).

-Michael
_________________________________________________________________
http://fastmail.ca/ - Fast Free Web Email for Canadians

From pgsql-patches-owner@postgresql.org Tue May 22 21:09:00 2001

Received: from candle.pha.pa.us (candle.navpoint.com [162.33.245.46])
by postgresql.org (8.11.3/8.11.1) with ESMTP id f4N0txA53485
for <pgsql-patches@postgresql.org>; Tue, 22 May 2001 20:55:59 -0400 (EDT)
(envelope-from pgman@candle.pha.pa.us)
Received: (from pgman@localhost)
by candle.pha.pa.us (8.10.1/8.10.1) id f4N0tpV17150;
Tue, 22 May 2001 20:55:51 -0400 (EDT)
From: Bruce Momjian <pgman@candle.pha.pa.us>
Message-Id: <200105230055.f4N0tpV17150@candle.pha.pa.us>
Subject: Re: Remove sort files
In-Reply-To: <3B0B08E6.000121.68010@frodo.searchcanada.ca> "from Michael Richards
at May 22, 2001 08:48:38 pm"
To: Michael Richards <michael@fastmail.ca>
Date: Tue, 22 May 2001 20:55:51 -0400 (EDT)
CC: pgsql-patches@postgresql.org
X-Mailer: ELM [version 2.4ME+ PL90 (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII
X-Archive-Number: 200105/160
X-Sequence-Number: 808

The only unusual part is the use of system("find...rm...") to
remove the files in pg_sorttemp. Seemed cleaner than doing fancy
directory walking code in C.

Should this figure out and use a path to the find command? Unless
postgres sets CWD, is it possible as-is to coerce it into rming some
files you did not intend to have rm'd? I didn't have/spend time
looking at it with a fine toothcomb, but usually calling find and rm
without an explicit path is a Bad Thing(tm).

I believe it is in /usr/bin on every OS, right? My guess is I have to
have configure find it. Let me get on that for 'find' and 'rm'. If
they are not found, I will skip it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

The only unusual part is the use of system("find...rm...") to
remove the files in pg_sorttemp. Seemed cleaner than doing fancy
directory walking code in C.

Should this figure out and use a path to the find command? Unless
postgres sets CWD, is it possible as-is to coerce it into rming some
files you did not intend to have rm'd? I didn't have/spend time
looking at it with a fine toothcomb, but usually calling find and rm
without an explicit path is a Bad Thing(tm).

I do use a full path for find:

find DataDir -...

Here is an updated patch which forces find and rm to come from /bin or
/usr/bin. Seems safer that way.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Bruce Momjian writes:

The only unusual part is the use of system("find...rm...") to remove the
files in pg_sorttemp. Seemed cleaner than doing fancy directory
walking code in C.

'find' is not in the portable subset of shell utilities, AFAIK. If you do
put them into a separate directory you could use "rm -rf %s" on that
directory.

Why don't we put the sort files into /tmp and let the system figure out
how to get rid of them?

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

Bruce Momjian writes:

The only unusual part is the use of system("find...rm...") to remove the
files in pg_sorttemp. Seemed cleaner than doing fancy directory
walking code in C.

'find' is not in the portable subset of shell utilities, AFAIK. If you do
put them into a separate directory you could use "rm -rf %s" on that
directory.

I did it that way so if pg_sorttemp is a symlink, I don't remove the
symlink, just the files inside. The 'find' needs to be updated though
to '! -file f' rather than '-type d' because it will not see symlinks,
and I am not sure the find symlink find flag is portable.

Anyway, I am still thinking about this. I haven't addressed tables that
are orphaned by a rename aborting.

I am no where near ready to apply this yet.

Why don't we put the sort files into /tmp and let the system figure out
how to get rid of them?

I am concerned that /tmp has size limitations that /data doesn't. I
know on my BSD/OS, it limits to 75MB because it is a memory-based file
system.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Here is an updated patch which forces find and rm to come from /bin or
/usr/bin. Seems safer that way.

createdb and dropdb assume that they should use cp and rm as found in
the postmaster's PATH. I see no good reason why this code shouldn't
behave the same.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Here is an updated patch which forces find and rm to come from /bin or
/usr/bin. Seems safer that way.

createdb and dropdb assume that they should use cp and rm as found in
the postmaster's PATH. I see no good reason why this code shouldn't
behave the same.

OK. I thought people would think my use of find and rm in the
postmaster code to be terrible, but I guess not. PATH stuff removed.

Again, this is a work in progress.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Peter Eisentraut <peter_e@gmx.net> writes:

Why don't we put the sort files into /tmp and let the system figure out
how to get rid of them?

Hard-wiring the assumption that they should be in /tmp would be VASTLY
worse than our current situation. Lots of people run with /tmp its own,
not very large partition. There are also serious security issues on
machines that don't have directory stickybits.

I do not like the way Bruce is doing the find, either, but moving stuff
to /tmp isn't the answer.

One reasonable idea is

cd pg_sorttemp ; rm -f *

which could potentially fail if there are LOTS of dead temp files
(enough to make the expanded rm command overrun your kernel limit on
command length) but if there are that many then you've got serious
problems anyway.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Anyway, I am still thinking about this. I haven't addressed tables that
are orphaned by a rename aborting.

What makes you think that needs to be addressed? We don't have that
problem anymore AFAIK.

regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:

Why don't we put the sort files into /tmp and let the system figure out
how to get rid of them?

Hard-wiring the assumption that they should be in /tmp would be VASTLY
worse than our current situation. Lots of people run with /tmp its own,
not very large partition. There are also serious security issues on
machines that don't have directory stickybits.

I do not like the way Bruce is doing the find, either, but moving stuff
to /tmp isn't the answer.

One reasonable idea is

cd pg_sorttemp ; rm -f *

which could potentially fail if there are LOTS of dead temp files
(enough to make the expanded rm command overrun your kernel limit on
command length) but if there are that many then you've got serious
problems anyway.

OK, here is the new 'find' line:

"find \"%s\"/base -name pg_sorttemp ! -type f -exec /bin/sh -c \"cd {}; rm -f *\" \\;",

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Anyway, I am still thinking about this. I haven't addressed tables that
are orphaned by a rename aborting.

What makes you think that needs to be addressed? We don't have that
problem anymore AFAIK.

If one backend creates a new file for alter table, but crashes before
modifying pg_class, doesn't that file just hang around?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

One reasonable idea is
cd pg_sorttemp ; rm -f *

OK, here is the new 'find' line:

No, the point is not to use find at all.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Anyway, I am still thinking about this. I haven't addressed tables that
are orphaned by a rename aborting.

What makes you think that needs to be addressed? We don't have that
problem anymore AFAIK.

If one backend creates a new file for alter table, but crashes before
modifying pg_class, doesn't that file just hang around?

ALTER TABLE doesn't create any new files.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

One reasonable idea is
cd pg_sorttemp ; rm -f *

OK, here is the new 'find' line:

No, the point is not to use find at all.

Suppose I'm creating temporary files to sort, on the supported
platforms (anything non-windows AFAIK) can't you just create and
unlink the file so long as you don't close it?

-Michael
_________________________________________________________________
http://fastmail.ca/ - Fast Free Web Email for Canadians

From pgsql-patches-owner@postgresql.org Wed May 23 14:17:20 2001

Received: from candle.pha.pa.us (candle.navpoint.com [162.33.245.46])
by postgresql.org (8.11.3/8.11.1) with ESMTP id f4NHrBA88002
for <pgsql-patches@postgresql.org>; Wed, 23 May 2001 13:53:11 -0400 (EDT)
(envelope-from pgman@candle.pha.pa.us)
Received: (from pgman@localhost)
by candle.pha.pa.us (8.10.1/8.10.1) id f4NHqrs21595;
Wed, 23 May 2001 13:52:53 -0400 (EDT)
From: Bruce Momjian <pgman@candle.pha.pa.us>
Message-Id: <200105231752.f4NHqrs21595@candle.pha.pa.us>
Subject: Re: Remove sort files
In-Reply-To: <15958.990639731@sss.pgh.pa.us> "from Tom Lane at May 23, 2001 01:42:11
pm"
To: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 23 May 2001 13:52:53 -0400 (EDT)
CC: Peter Eisentraut <peter_e@gmx.net>,
PostgreSQL-patches <pgsql-patches@postgresql.org>
X-Mailer: ELM [version 2.4ME+ PL90 (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII
X-Archive-Number: 200105/171
X-Sequence-Number: 820

Bruce Momjian <pgman@candle.pha.pa.us> writes:

One reasonable idea is
cd pg_sorttemp ; rm -f *

OK, here is the new 'find' line:

No, the point is not to use find at all.

Actually, I can do:

rm -f */pg_sorttemp/*

New command is:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"rm -f \"%s\"/base/*/pg_sorttemp/*",
DataDir);

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

"Michael Richards" <michael@fastmail.ca> writes:

Suppose I'm creating temporary files to sort, on the supported
platforms (anything non-windows AFAIK) can't you just create and
unlink the file so long as you don't close it?

No.

(A) cygwin is considered a supported platform.

(B) we use virtual-file references to these files; there is no guarantee
that the file will be held open continuously for as long as we need it.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

One reasonable idea is
cd pg_sorttemp ; rm -f *

OK, here is the new 'find' line:

No, the point is not to use find at all.

Actually, I can do:

rm -f */pg_sorttemp/*

New command is:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"rm -f \"%s\"/base/*/pg_sorttemp/*",
DataDir);

I said cd for a reason: if you do it that way, you've cut the maximum
number of files you can delete by at least an order of magnitude,
depending on how long DataDir is.

Why do we need per-database sorttemp areas anyway? Seems better to have
an installation-wide one.

regards, tom lane

Bruce Momjian writes:

New command is:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"rm -f \"%s\"/base/*/pg_sorttemp/*",
DataDir);

Why not put all sort files into a common directory and then do a simple
readdir() loop in C?

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.net> writes:

Bruce Momjian writes:

New command is:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"rm -f \"%s\"/base/*/pg_sorttemp/*",
DataDir);

Why not put all sort files into a common directory and then do a simple
readdir() loop in C?

I was about to say that the potential portability headaches would be
more trouble than the "feature" is worth. OTOH, I see that xlog.c is
depending on readdir already, so maybe it's OK to use it.

regards, tom lane

New command is:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"rm -f \"%s\"/base/*/pg_sorttemp/*",
DataDir);

I said cd for a reason: if you do it that way, you've cut the maximum
number of files you can delete by at least an order of magnitude,
depending on how long DataDir is.

Why do we need per-database sorttemp areas anyway? Seems better to have
an installation-wide one.

OK, here is the new code:

snprintf(clear_pg_sorttemp, sizeof(clear_pg_sorttemp),
"sh -c '\
cd \"%s\"/base && \
ls | while read DIR; \
do \
export DIR; \
(cd \"$DIR\"/pg_sorttemp/ 2>/dev/null && rm -f *); \
done'",
DataDir);

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Anyway, I am still thinking about this. I haven't addressed tables that
are orphaned by a rename aborting.

What makes you think that needs to be addressed? We don't have that
problem anymore AFAIK.

If one backend creates a new file for alter table, but crashes before
modifying pg_class, doesn't that file just hang around?

ALTER TABLE doesn't create any new files.

What about CLUSTER? If we do DROP COLUMN by creating a new heap, we
will use it then too, right? Are those the only two that create new
files that could be orphaned?

Now that we have file numbers, seems we can do DROP COLUMN now reliably,
right?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@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