Thread: Re: [HACKERS] v6.5 release ToDo
> Hey, > Got a couple of problems that could hopefully be fixed before 6.5 gets > released. This is straight from CVS april 5th. > > Prob #1: > DROP TABLE <table> doesn't removed "extended" files. > i.e., if you have 2GB db on Linux(i386), it will only delete the main > file and not the .1, .2, etc table files. Added to TODO list. > > Prob #2: > While running postmaster at the command line like this: > /home/postgres/bin/postmaster -B 1024 -D/home/postgres/data -d 9 -o "-S > 4096 -s -d 9 -A" > > the current backend for the following CREATE TABLE would > die(consistently): > CREATE TABLE oletest ( > id serial, > number int, > string varchar(255) > ); > Not sure how to address this. > This was the only query it died on however. I have made huge indexes and > regular selects while running it the same way. There was only one > postgres backend running at the time. > > Thanks, > Ole Gjerde > > > > > -- 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, Pennsylvania19026
> > Prob #1: > > DROP TABLE <table> doesn't removed "extended" files. > > i.e., if you have 2GB db on Linux(i386), it will only delete the main > > file and not the .1, .2, etc table files. I have looked at this. I made it so it rolled over files at 1MB. My table ended up with 120 segments, and my indexes had 3(Yes, it DOES work!). DROP TABLE removed ALL segments from the table, but only the main index segment. So it looks like removing the table itself is using mdunlink in md.c, while removing indexes uses FileNameUnlink() which only unlinks 1 file. As far as I can tell, calling FileNameUnlink() and mdunlink() is basically the same, except mdunlink() deletes any extra segments. I've done some testing and it seems to work. It also passes regression tests(except float8, geometry and rules, but that's normal). If this patch is right, this fixes all known multi-segment problems on Linux. Ole Gjerde Patch for index drop: --- src/backend/catalog/index.c 1999/05/10 00:44:55 1.71 +++ src/backend/catalog/index.c 1999/05/15 06:42:27 @@ -1187,7 +1187,7 @@ */ ReleaseRelationBuffers(userindexRelation); - if (FileNameUnlink(relpath(userindexRelation->rd_rel->relname.data)) < 0) + if (mdunlink(userindexRelation) != SM_SUCCESS) elog(ERROR, "amdestroyr: unlink: %m"); index_close(userindexRelation);
Applied. You have the correct patch. All other references to FileNameUnlink look correct, but the index.c one is clearly wrong. Thanks. I believe we still have vacuuming of multi-segment tables as a problem. > > > Prob #1: > > > DROP TABLE <table> doesn't removed "extended" files. > > > i.e., if you have 2GB db on Linux(i386), it will only delete the main > > > file and not the .1, .2, etc table files. > > I have looked at this. > I made it so it rolled over files at 1MB. My table ended up with 120 > segments, and my indexes had 3(Yes, it DOES work!). > DROP TABLE removed ALL segments from the table, but only the main index > segment. > > So it looks like removing the table itself is using mdunlink in md.c, > while removing indexes uses FileNameUnlink() which only unlinks 1 file. > As far as I can tell, calling FileNameUnlink() and mdunlink() is basically > the same, except mdunlink() deletes any extra segments. > > I've done some testing and it seems to work. It also passes regression > tests(except float8, geometry and rules, but that's normal). > > If this patch is right, this fixes all known multi-segment problems on > Linux. > > Ole Gjerde > > Patch for index drop: > --- src/backend/catalog/index.c 1999/05/10 00:44:55 1.71 > +++ src/backend/catalog/index.c 1999/05/15 06:42:27 > @@ -1187,7 +1187,7 @@ > */ > ReleaseRelationBuffers(userindexRelation); > > - if (FileNameUnlink(relpath(userindexRelation->rd_rel->relname.data)) < 0) > + if (mdunlink(userindexRelation) != SM_SUCCESS) > elog(ERROR, "amdestroyr: unlink: %m"); > > index_close(userindexRelation); > > > -- 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, Pennsylvania19026
I believe also we have: DROP TABLE/RENAME TABLE doesn't remove extended files, *.1, *.2 as an open item. Do you see these problems there? > > > Prob #1: > > > DROP TABLE <table> doesn't removed "extended" files. > > > i.e., if you have 2GB db on Linux(i386), it will only delete the main > > > file and not the .1, .2, etc table files. > > I have looked at this. > I made it so it rolled over files at 1MB. My table ended up with 120 > segments, and my indexes had 3(Yes, it DOES work!). > DROP TABLE removed ALL segments from the table, but only the main index > segment. > > So it looks like removing the table itself is using mdunlink in md.c, > while removing indexes uses FileNameUnlink() which only unlinks 1 file. > As far as I can tell, calling FileNameUnlink() and mdunlink() is basically > the same, except mdunlink() deletes any extra segments. > > I've done some testing and it seems to work. It also passes regression > tests(except float8, geometry and rules, but that's normal). > > If this patch is right, this fixes all known multi-segment problems on > Linux. > > Ole Gjerde > > Patch for index drop: > --- src/backend/catalog/index.c 1999/05/10 00:44:55 1.71 > +++ src/backend/catalog/index.c 1999/05/15 06:42:27 > @@ -1187,7 +1187,7 @@ > */ > ReleaseRelationBuffers(userindexRelation); > > - if (FileNameUnlink(relpath(userindexRelation->rd_rel->relname.data)) < 0) > + if (mdunlink(userindexRelation) != SM_SUCCESS) > elog(ERROR, "amdestroyr: unlink: %m"); > > index_close(userindexRelation); > > > -- 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, Pennsylvania19026
On Sat, 15 May 1999, Bruce Momjian wrote:
> I believe also we have:
> DROP TABLE/RENAME TABLE doesn't remove extended files, *.1, *.2
> as an open item. Do you see these problems there?
DROP TABLE worked, ALTER TABLE didn't.
CREATE TABLE
DROP TABLE
CREATE INDEX
DROP INDEX
ALTER TABLE old RENAME TO new
All works on linux now by my tests and regression(with patch below).
Perhaps a mdrename() should be created? Or is something like this good
enough?
Another thing. Should error messages from file related(or all system
calls) use strerror() to print out errno?
Ole Gjerde
--- src/backend/commands/rename.c 1999/05/10 00:44:59 1.23
+++ src/backend/commands/rename.c 1999/05/15 23:42:49
@@ -201,10 +201,13 @@voidrenamerel(char *oldrelname, char *newrelname){
+ int i; Relation relrelation; /* for RELATION relation */ HeapTuple oldreltup; char
oldpath[MAXPGPATH],
- newpath[MAXPGPATH];
+ newpath[MAXPGPATH],
+ toldpath[MAXPGPATH + 10],
+ tnewpath[MAXPGPATH + 10]; Relation irelations[Num_pg_class_indices]; if
(!allowSystemTableMods&& IsSystemRelationName(oldrelname))
@@ -229,6 +232,14 @@ strcpy(newpath, relpath(newrelname)); if (rename(oldpath, newpath) < 0) elog(ERROR,
"renamerel:unable to rename file: %s", oldpath);
+
+ for (i = 1;; i++)
+ {
+ sprintf(toldpath, "%s.%d", oldpath, i);
+ sprintf(tnewpath, "%s.%d", newpath, i);
+ if(rename(toldpath, tnewpath) < 0)
+ break;
+ } StrNCpy((((Form_pg_class) GETSTRUCT(oldreltup))->relname.data), newrelname, NAMEDATALEN);
> On Sat, 15 May 1999, Bruce Momjian wrote:
> > I believe also we have:
> > DROP TABLE/RENAME TABLE doesn't remove extended files, *.1, *.2
> > as an open item. Do you see these problems there?
>
> DROP TABLE worked, ALTER TABLE didn't.
>
> CREATE TABLE
> DROP TABLE
> CREATE INDEX
> DROP INDEX
> ALTER TABLE old RENAME TO new
>
> All works on linux now by my tests and regression(with patch below).
Applied.
>
> Perhaps a mdrename() should be created? Or is something like this good
> enough?
I think this is good enough for now. Do people want an mdrename?
>
> Another thing. Should error messages from file related(or all system
> calls) use strerror() to print out errno?
>
Seems like in the code you have, you just keep renaming until you can't
find any more files, so printing out any errno would be a problem,
right?
I assume you are taling about the initial rename. Not sure if strerror
would help. We really try and insulate the user from knowing how we are
doing the SQL we do, so it is possible it may be confusing. However, it
may be very helpful too. Not sure. Comments?
> Ole Gjerde
>
> --- src/backend/commands/rename.c 1999/05/10 00:44:59 1.23
> +++ src/backend/commands/rename.c 1999/05/15 23:42:49
> @@ -201,10 +201,13 @@
> void
> renamerel(char *oldrelname, char *newrelname)
> {
> + int i;
> Relation relrelation; /* for RELATION relation */
> HeapTuple oldreltup;
> char oldpath[MAXPGPATH],
> - newpath[MAXPGPATH];
> + newpath[MAXPGPATH],
> + toldpath[MAXPGPATH + 10],
> + tnewpath[MAXPGPATH + 10];
> Relation irelations[Num_pg_class_indices];
>
> if (!allowSystemTableMods && IsSystemRelationName(oldrelname))
> @@ -229,6 +232,14 @@
> strcpy(newpath, relpath(newrelname));
> if (rename(oldpath, newpath) < 0)
> elog(ERROR, "renamerel: unable to rename file: %s", oldpath);
> +
> + for (i = 1;; i++)
> + {
> + sprintf(toldpath, "%s.%d", oldpath, i);
> + sprintf(tnewpath, "%s.%d", newpath, i);
> + if(rename(toldpath, tnewpath) < 0)
> + break;
> + }
>
> StrNCpy((((Form_pg_class) GETSTRUCT(oldreltup))->relname.data),
> newrelname, NAMEDATALEN);
>
>
>
-- 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,
Pennsylvania19026