[PATCHES] Tablespaces
Hi all, Attached is my latest patch implementing tablespaces. This has all the functionality I was planning for 7.5. Most of the information about the patch is contained in the patch/documentation, previous submissions and the archives. Testing, review, comments would be greatly appreciated. Gavin tablespace-18.diff.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Tablespaces
Gavin Sherry wrote:
Attached is my latest patch implementing tablespaces. This has all the
functionality I was planning for 7.5.
A few minor points I happened to notice while reading through the patch:
+ * To simply initialisation and XLog activity, have create and
maintain
+ * a symbolic link map in data/pg_tablespaces.
Grammar errors.
+ void
+ TblspcCreateDbspace(Oid tbloid)
+ {
+ #ifndef HAVE_SYMLINK
+ return;
+ #endif
+ struct stat st;
+ char*dir;
If HAVE_SYMLINK is undefined, this is a syntax error (at least in
C89, which is what we ought to limit ourselves to). Similar problems
elsewhere in the same file (tablespc.c)
+ dir = (char *) palloc(strlen(DataDir) + 14 + 10 + 10 + 10 + 3 + 1);
+ sprintf(dir, %s/pg_tablespaces/%u/%u, DataDir, tbloid,
+ MyDatabaseId);
Is the length of that buffer right? At the least the addition is a
little weird (why are you adding 10 three times for two numeric
variables?) I noticed another buffer allocation (linkloc) that
looked dubious at first glance as well.
+ charrealnewpath[MAXPGPATH];
This is somewhat pedantic, but how do we know that MAXPGPATH =
PATH_MAX (the minimum safe size of the second argument to
realpath(), at least on my local system)?
-Neil
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Relocatable locale
Am Donnerstag, 27. Mai 2004 13:05 schrieb Magnus Hagander: I don't beleive this is a windows-specific issue. It's a general issue for all relocateable installs. Except that no one except Windows uses relocatable installs. As for how to do it - on Windows you *can* get the path of the DLL that is executing your code, using GetModuleFileName(). Hardly cross-platform, but can be done. That sounds pretty reasonable to me. ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Relocatable locale
Am Donnerstag, 27. Mai 2004 07:31 schrieb Bruce Momjian: Sure, it is a hack, but what other option do we have? Do we somehow move the locale files into the library? On Windows we use the feature that DLLs can locate themselves (described by Magnus Hagander), and on Unix we forget about pretending to offer relocatable installations. ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] Relocatable locale
Am Donnerstag, 27. Mai 2004 14:41 schrieb Bruce Momjian: As for how to do it - on Windows you *can* get the path of the DLL that is executing your code, using GetModuleFileName(). Hardly cross-platform, but can be done. That sounds pretty reasonable to me. True, and we already have our own find_my_exec() which works for Unix too. What does that have to do with this case? We're trying to find the library here. ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Relocatable locale
Peter Eisentraut wrote: Am Donnerstag, 27. Mai 2004 14:41 schrieb Bruce Momjian: As for how to do it - on Windows you *can* get the path of the DLL that is executing your code, using GetModuleFileName(). Hardly cross-platform, but can be done. That sounds pretty reasonable to me. True, and we already have our own find_my_exec() which works for Unix too. What does that have to do with this case? We're trying to find the library here. Oh, I see that GetModuleFileName() finds the location of your library, not of the binary. Nice. So, for Win32, we use that function call to find the locale directory? We do have a comment in port/exec.c: if (GetModuleFileName(NULL, retpath, MAXPGPATH) == 0) and I thought that did only binaries, not the library that uses them. I assume if the library is a DLL, it returns the DLL location, and if it is in the binary, it returns the binary location. I will make a Win32-specific patch. Do we want an evironment variable for Unix? Seems folks do. I will work on that too. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Relocatable locale
Am Donnerstag, 27. Mai 2004 15:03 schrieb Bruce Momjian: I will make a Win32-specific patch. Do we want an evironment variable for Unix? Seems folks do. I will work on that too. The gettext library already has an environment variable for that (NLSPATH I think). I don't know if we need a PostgreSQL-specific one. ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Relocatable locale
As for how to do it - on Windows you *can* get the path of the DLL that is executing your code, using GetModuleFileName(). Hardly cross-platform, but can be done. That sounds pretty reasonable to me. True, and we already have our own find_my_exec() which works for Unix too. What does that have to do with this case? We're trying to find the library here. Oh, I see that GetModuleFileName() finds the location of your library, not of the binary. Nice. So, for Win32, we use that function call to find the locale directory? We do have a comment in port/exec.c: if (GetModuleFileName(NULL, retpath, MAXPGPATH) == 0) and I thought that did only binaries, not the library that uses them. I assume if the library is a DLL, it returns the DLL location, and if it is in the binary, it returns the binary location. Nope, not quite. With the parameter NULL, it will return for the current *process*. You will need to use GetModuleHandle() (I think that should work) to get the handle of the DLL. See http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dllproc /base/getmodulehandle.asp For static libaries, you need a different solution. Base that off the EXE? //Magnus ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Relocatable locale
Bruce Momjian [EMAIL PROTECTED] writes: I will make a Win32-specific patch. Do we want an evironment variable for Unix? No, we do not. This entire thread has consisted of everyone but you objecting loudly to that idea. regards, tom lane ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Relocatable locale
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I will make a Win32-specific patch. Do we want an evironment variable for Unix? No, we do not. This entire thread has consisted of everyone but you objecting loudly to that idea. They didn't sound very loud to me. Why not have it so Unix folks can control it? Also, how do we get our native apps to know the location of the locale directory? The only other solution I can think of is a libpq function call to set it, but of course that can't be changed once the binary is created. Our apps could use the function call (based on the bin location), and other apps would have to decide how they would make this functionality visible, but it wouldn't be consistent. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Cancel/Kill backend functions
Arrgh, when will I ever learn :-( Attached. //Magnus -Original Message- From: Bruce Momjian [mailto:[EMAIL PROTECTED] Sent: den 26 maj 2004 20:50 To: Magnus Hagander Cc: Neil Conway; [EMAIL PROTECTED] Subject: Re: [PATCHES] Cancel/Kill backend functions Magnus, would you please resumbit this as a context diff? --- Magnus Hagander wrote: Okay, here is an updated patch. now uses IsBackendPid(), which is closely modeled (read cut-and-pasted) from TransactionIdIsInProgress(). Since it's no longer a pgstat function, I moved it to misc.c. Not 100% that's the right place either, but it seemed like the best alternative. //Magnus -Original Message- From: Neil Conway [mailto:[EMAIL PROTECTED] Sent: den 22 maj 2004 10:00 To: Magnus Hagander Cc: [EMAIL PROTECTED] Subject: Re: [PATCHES] Cancel/Kill backend functions Magnus Hagander wrote: Per previous discussions, here are two functions to send INT and TERM signals to other backends.They permit only INT and TERM, and permits sending only to postgresql backends (as registered in pgstat). Why does this depend on pgstat? ISTM it would be better to use the per-backend PGPROC information, which is stored in shared memory. Consider TransactionIdIsInProgress() for an example. -Neil Content-Description: termbackend.patch [ Attachment, skipping... ] ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 termbackend2.patch Description: termbackend2.patch ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Cancel/Kill backend functions
On Thu, May 27, 2004 at 08:08:34PM +0200, Magnus Hagander wrote: Magnus Hagander wrote: Okay, here is an updated patch. now uses IsBackendPid(), which is closely modeled (read cut-and-pasted) from TransactionIdIsInProgress(). I wonder what can happen if a backend passes the IsBackendPid() test and terminates just before the kill() signal? It should be pretty unlikely but you could signal the wrong process ... shouldn't the SInvalLock be held throughout the whole operation? -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) El número de instalaciones de UNIX se ha elevado a 10, y se espera que este número aumente (UPM, 1972) ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Cancel/Kill backend functions
Okay, here is an updated patch. now uses IsBackendPid(), which is closely modeled (read cut-and-pasted) from TransactionIdIsInProgress(). I wonder what can happen if a backend passes the IsBackendPid() test and terminates just before the kill() signal? It should be pretty unlikely but you could signal the wrong process ... shouldn't the SInvalLock be held throughout the whole operation? You'd actually need to get a pid *reuse* during that short time. Otherwise, you're just kill():ing a nonexistant process, which should be no problem. This is the same as issuing a kill -INT pid from the shell after doing ps(1), which is basically what this function tries to emulate. Should be no more dangerous than that. Bottom line - while maybe slightly more correcet, not sure it's necessary. //Magnus ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] Cancel/Kill backend functions
Magnus Hagander wrote: You'd actually need to get a pid *reuse* during that short time. That isn't so implausible on a system which assigns PIDs randomly. Holding the SInvalLock doesn't remove the race condition, but it makes it less likely to occur for essentially very little cost. Bottom line - while maybe slightly more correcet, not sure it's necessary. IMHO it's worth doing. -Neil ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] win32 locale fixes
Magnus Hagander [EMAIL PROTECTED] writes: The following patch fixes locale support under win32. I've applied the change in pg_locale.c, and will fold the other changes into an upcoming rewrite of the EXEC_BACKEND code. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] [HACKERS] Configuration patch
I am working on integrating this patch now.
What is the logic if checkConfigDir(). It seems incompleted or wrong.
Your code had:
+ if(S_ISREG(stat_buf.st_mode)) /* It's a regular file, so assume it's
an explict */
+ {
+ return TRUE;
+ }
+ else if(S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it a config or
system dir */
+ {
+ snprintf(path, sizeof(path), %s/global/pg_control, checkdir);
+ /* If this is not found, then, hey it wasn't going to work as a data
dir either. */
+ if(stat(path, stat_buf) == -1)
+ return TRUE;
+ }
+ return FALSE;
The last stat() seems to say that if the directory has no
global/pg_control, then it is good a a config-only directory, and if not
it is a data directory. Is that right?
---
[EMAIL PROTECTED] wrote:
This patch incorporates a number of changes suggested by the group. The
purpose of this patch is to move postgresql to a position where all
configuration options are specified in one place. The postgresql.conf file
could completely document a postgresql environment.
It adds this functionality:
The -D' option will work as it always has if it is set to a standard
postgresql database cluster directory. If it is set to a postgresql.conf
file, it will use that file for configuration. If it is set to a directory
which is not a cluster directory, i.e. /somepath/etc it will look for
pg_hba.conf, pg_ident.conf, and postgresql.conf there.
For postgresql to work only with a configuration file, some options have
been added:
include = '/etc/postgres/debug.conf'
pgdata = '/vol01/postgres'
hba_conf = '/etc/postgres/pg_hba_conf'
ident_conf = '/etc/postgres/pg_ident.conf'
runtime_pidfile = '/var/run/postgresql.conf'
tablespace = '/somevol/somepath'
include allows files with configuration parameters to be included.
pgdata (used to be data_dir in old patch) tells PostgreSQL where it's
database cluster directory is located.
hba_conf tells PostgreSQL where to find pg_hba.conf file.
ident_conf tells PostgreSQL where to find pg_ident.conf.
runtime_pidfile tells postgres to write it's PID to a file that would be
used by external applications. It is *NOT* the pid file which postgresql
uses.
tablespace allows postgresql to use alternate locations without
environment variables. Using SIGHUP, tablespaces are reloaded. This allows
you to add tablespaces to a running PostgreSQL process. (I know this has a
limited lifetime, but it may make CREATE DATABASE ... WITH LOCATION a
little bit more sane in the meantime.
[ Attachment, skipping... ]
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania 19073
---(end of broadcast)---
TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Configuration patch
I am working on integrating this patch now.
What is the logic if checkConfigDir(). It seems incompleted or wrong.
Your code had:
+ if(S_ISREG(stat_buf.st_mode)) /* It's a regular file, so
assume it's an explict */
+ {
+ return TRUE;
+ }
+ else if(S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it
a config or system dir */
+ {
+ snprintf(path, sizeof(path), %s/global/pg_control,
checkdir);
+ /* If this is not found, then, hey it wasn't going to work
as a data dir either. */
+ if(stat(path, stat_buf) == -1)
+ return TRUE;
+ }
+ return FALSE;
The last stat() seems to say that if the directory has no
global/pg_control, then it is good a a config-only directory, and if not
it is a data directory. Is that right?
Yes, that is right. If pg_control exists in the directory, then we
should not assume it is a config-only directory.
---
[EMAIL PROTECTED] wrote:
This patch incorporates a number of changes suggested by the group. The
purpose of this patch is to move postgresql to a position where all
configuration options are specified in one place. The postgresql.conf
file
could completely document a postgresql environment.
It adds this functionality:
The -D' option will work as it always has if it is set to a standard
postgresql database cluster directory. If it is set to a
postgresql.conf
file, it will use that file for configuration. If it is set to a
directory
which is not a cluster directory, i.e. /somepath/etc it will look for
pg_hba.conf, pg_ident.conf, and postgresql.conf there.
For postgresql to work only with a configuration file, some options have
been added:
include = '/etc/postgres/debug.conf'
pgdata = '/vol01/postgres'
hba_conf = '/etc/postgres/pg_hba_conf'
ident_conf = '/etc/postgres/pg_ident.conf'
runtime_pidfile = '/var/run/postgresql.conf'
tablespace = '/somevol/somepath'
include allows files with configuration parameters to be included.
pgdata (used to be data_dir in old patch) tells PostgreSQL where it's
database cluster directory is located.
hba_conf tells PostgreSQL where to find pg_hba.conf file.
ident_conf tells PostgreSQL where to find pg_ident.conf.
runtime_pidfile tells postgres to write it's PID to a file that would
be
used by external applications. It is *NOT* the pid file which postgresql
uses.
tablespace allows postgresql to use alternate locations without
environment variables. Using SIGHUP, tablespaces are reloaded. This
allows
you to add tablespaces to a running PostgreSQL process. (I know this has
a
limited lifetime, but it may make CREATE DATABASE ... WITH LOCATION a
little bit more sane in the meantime.
[ Attachment, skipping... ]
---(end of broadcast)---
TIP 6: Have you searched our list archives?
http://archives.postgresql.org
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup.| Newtown Square, Pennsylvania
19073
---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] Tablespaces
Attached is an updated patch, fixing a compile error which my compiler didn't seem to detect/suffer from and incorporating fixes to problems raised by Neil. Thanks, Gavin tablespace-20.diff.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Cancel/Kill backend functions
On Fri, May 28, 2004 at 01:01:10AM -0400, Tom Lane wrote: Neil Conway [EMAIL PROTECTED] writes: Magnus Hagander wrote: You'd actually need to get a pid *reuse* during that short time. That isn't so implausible on a system which assigns PIDs randomly. Holding the SInvalLock doesn't remove the race condition, but it makes it less likely to occur for essentially very little cost. Other than holding a fairly critical lock for an operation that will take an unknown amount of time. With this comment, I take it you'd disagree with my recoding of TransactionIdIsCurrentTransactionId(). The current code has to scan only the xid's in each PGPROC struct. However I had to rewrite it to peek at pg_subtrans, and this is done while the SInvalLock is share-locked. pg_subtrans may need to do some I/O to get the data, and there could be multiple queries, depending on the nesting level. I could write it to save the xid's in PGPROC in a first pass, then release the SInvalLock, then look at pg_subtrans. But I think doing it this way has a (is a?) race condition. -- Alvaro Herrera (alvherre[a]dcc.uchile.cl) Bob [Floyd] used to say that he was planning to get a Ph.D. by the green stamp method, namely by saving envelopes addressed to him as 'Dr. Floyd'. After collecting 500 such letters, he mused, a university somewhere in Arizona would probably grant him a degree. (Don Knuth) ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[PATCHES] Small doc patch for area() function...
Small patch that adds some documentation for the area() function. Specifically, point out that intersecting points in a path will yield (most likely), unexpected results. Visually these are identical paths, but mathematically they're not the same. Ex: area | plan -- +--- --- -0 | ((0,0),(0,1),(2,1),(2,2),(1,2),(1,0),(0,0)) 2 | ((0,0),(0,1),(1,1),(1,2),(2,2),(2,1),(1,1),(1,0),(0,0)) The current algorithm for area(PATH) is very quick, but only handles non-intersecting paths. I'm going to work on two other functions for the PATH data type that determines if a PATH is intersecting or not, and a function that returns the area() for an intersecting PATH. The intersecting area() function will be considerably slower (I think it's going to be O(n!) or worse instead of the current O(n), but that comes with the territory). -sc Index: func.sgml === RCS file: /projects/cvsroot/pgsql-server/doc/src/sgml/func.sgml,v retrieving revision 1.205 diff -u -r1.205 func.sgml --- func.sgml 26 May 2004 18:35:31 - 1.205 +++ func.sgml 28 May 2004 04:49:43 - @@ -5971,6 +5971,22 @@ as an array of two typepoint/ values. /para +para + The functionarea/function function works for the types + typebox/type, typecircle/type, and typepath/type. + The functionarea/function function only works on the + typepath/type data type if the points in the + typepath/type are non-intersecting. For example, the + typepath/type + literal'((0,0),(0,1),(2,1),(2,2),(1,2),(1,0),(0,0))'::PATH/literal + won't work, however, the following visually identical + typepath/type + literal'((0,0),(0,1),(1,1),(1,2),(2,2),(2,1),(1,1),(1,0),(0,0))'::PATH/literal + will work. If the concept of an intersecting versus + non-intersecting typepath/type is confusing, draw both of the + above typepath/types side by side on a piece of graph paper. +/para + /sect1 PS Right now I'm developing on OS-X and there's a geometry regression test that's returning -0. FWIW *** ./expected/geometry.out Fri Oct 31 19:07:07 2003 --- ./results/geometry.out Thu May 27 22:16:58 2004 *** *** 117,123 | (5.1,34.5) | [(1,2),(3,4)] | (3,4) | (-5,-12) | [(1,2),(3,4)] | (1,2) | (10,10)| [(1,2),(3,4)] | (3,4) ! | (0,0) | [(0,0),(6,6)] | (-0,0) | (-10,0)| [(0,0),(6,6)] | (0,0) | (-3,4) | [(0,0),(6,6)] | (0.5,0.5) | (5.1,34.5) | [(0,0),(6,6)] | (6,6) --- 117,123 | (5.1,34.5) | [(1,2),(3,4)] | (3,4) | (-5,-12) | [(1,2),(3,4)] | (1,2) | (10,10)| [(1,2),(3,4)] | (3,4) ! | (0,0) | [(0,0),(6,6)] | (0,0) | (-10,0)| [(0,0),(6,6)] | (0,0) | (-3,4) | [(0,0),(6,6)] | (0.5,0.5) | (5.1,34.5) | [(0,0),(6,6)] | (6,6) == -- Sean Chittenden ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
