Re: [PATCHES] libpq Win32 Mutex performance patch
Andrew Chernow wrote: Magnus Hagander wrote: It changes the behavior when the pointer passed in is invalid from crash to silent working, right? Correct, it a Habit. I sub-consciously write code that checks pointers. We can remove the pointer checks and let the thing dump core if people prefer. Actually, if we can avoid it being a pointer at all, that'd be even better :-) Because if we don't send a pointer we have to allocate, then the critical section functions have the same properties as the pthread ones, namely they cannot fail. Any chance for that way instead? //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq Win32 Mutex performance patch
Andrew Chernow wrote: Tom Lane wrote: Andrew Chernow [EMAIL PROTECTED] writes: The attached patch replaces the win32 mutex calls with critical section calls. The change will not affect the behavior of the windows pthread_xxx functions. Why have you defined the lock/unlock functions as willing to fall through silently if handed a null pointer? I think a crash in such a case is what we *want*. Silently not locking is surely not very safe. regards, tom lane Yeah, both naughty. These functions were not implemented to spec. These pthread functions are all supposed to return an int (which is an errno value). I was trying not to change the existing prototypes ... should I? I can return EINVAL if something is NULL and ENOMEM if the malloc fails ... or just dump core. If you like the return value idea, I can make all occurances of pthread_xxx check the return value. Getting these emails in the wrong order for some reason :-( Yes, actually checking the return values from these functions in libpq seems like a good idea to me. ISTM that we already have the case where we can fall through silently when failing to lock, and that should be fixed. It looks like the internal API of pgthreadlock_t needs to be changed as well in this case, because it can currently only reaturn void. But - it also seems we are not actually *using* it anywhere. Perhaps that part of the API should simply be removed? //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] Show INHERIT in \du
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sun, Apr 13, 2008 at 6:11 PM, Brendan Jurd wrote: I've written a patch which implements the same \du behaviour as my previous patch, but using the new printTable API New versions of this patch, including changes made to the printTable API in [1] As before, I've included a patch against HEAD which includes the printTable changes (du-attributes-print-table_2.diff.bz2), and a patch against my printTable branch which shows just the changes to describeRoles (du-attributes_2.diff.bz2). Cheers, BJ [1] http://archives.postgresql.org/message-id/[EMAIL PROTECTED] -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFIBGZP5YBsbHkuyV0RAsb9AKDrFJ/8V00t2XCwIihzEZYcPQZKiQCg3q6L RkiMfjqLay/JLk8phnniYLs= =oW6S -END PGP SIGNATURE- du-attributes-print-table_2.diff.bz2 Description: BZip2 compressed data du-attributes_2.diff.bz2 Description: BZip2 compressed data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] printTable API (was: Show INHERIT in \du)
On Tue, Apr 15, 2008 at 06:11:32PM +1000, Brendan Jurd wrote: Now my question is, how do I indicate that a second version of the patch has been submitted on the wiki? Should I leave the primary link pointing at the original submission, or update it to point at this message? I'd say add a row: New version submitted (link) (brief summary of changes) If there are many more versions you can consider cutting, but you're not there yet. Have a nice day, -- Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/ Please line up in a tree and maintain the heap invariant while boarding. Thank you for flying nlogn airlines. signature.asc Description: Digital signature
Re: [PATCHES] libpq Win32 Mutex performance patch
Merlin Moncure wrote: On Fri, Apr 11, 2008 at 2:49 PM, Magnus Hagander [EMAIL PROTECTED] wrote: Andrew Chernow wrote: I noticed several months ago, and came across it again today, that libpq's pthread-win32.c implementation is using CreateMutex rather than CRITICAL_SECTION. CreateMutex is like a semaphore in that it is designed to be accessible via name system-wide. Even when you don't give it a name, thus bound to process that created it, it still carries significant overhead compared to using win32 CRITICAL_SECTIONs. The attached patch replaces the win32 mutex calls with critical section calls. The change will not affect the behavior of the windows pthread_xxx functions. First of all, I like this in general :-) But a couple of comments. It changes the behavior when the pointer passed in is invalid from crash to silent working, right? This shouldn't actually matter, since these functions are only ever supposed to run from callers *inside libpq*, so it probalby doesn't matter... I noticed you conjured up a ecpg threading patch sometime around early 2007. You used a mutex there deliberately because that's what libpq did. Maybe that patch should be adjusted? Yes, I think it should. //Magnus -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Sorting writes during checkpoint
Here is a patch for TODO item, Consider sorting writes during checkpoint. It writes dirty buffers in the order of block number during checkpoint so that buffers are written sequentially. I proposed the patch before, but it was rejected because 8.3 feature has been frozen already at that time. http://archives.postgresql.org/pgsql-hackers/2007-06/msg00541.php I rewrote it to be applied cleanly against current HEAD, but the concept is not changed at all -- Memorizing pairs of (buf_id, BufferTag) for each dirty buffer into an palloc-ed array at the start of checkpoint. Sorting the array in BufferTag order and writing buffers in the order. There are 10% of performance win in pgbench on my machine with RAID-0 disks. There can be more benefits on RAID-5 disks, because random writes are slower than sequential writes there. [HEAD] tps = 1134.233955 (excluding connections establishing) [HEAD with patch] tps = 1267.446249 (excluding connections establishing) [pgbench] transaction type: TPC-B (sort of) scaling factor: 100 query mode: simple number of clients: 32 number of transactions per client: 10 number of transactions actually processed: 320/320 [hardware] 2x Quad core Xeon, 16GB RAM, 4x HDD (RAID-0) [postgresql.conf] shared_buffers = 2GB wal_buffers = 4MB checkpoint_segments = 64 checkpoint_timeout = 5min checkpoint_completion_target = 0.5 Regards, --- ITAGAKI Takahiro NTT Open Source Software Center sorted-ckpt-84.patch Description: Binary data -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Sorting writes during checkpoint
On Tue, 15 Apr 2008, ITAGAKI Takahiro wrote: 2x Quad core Xeon, 16GB RAM, 4x HDD (RAID-0) What is the disk controller in this system? I'm specifically curious about what write cache was involved, so I can get a better feel for the hardware your results came from. I'm busy rebuilding my performance testing systems right now, once that's done I can review this on a few platforms. One thing that jumped out at me just reading the code is this happening inside BufferSync: buf_to_write = (BufAndTag *) palloc(NBuffers * sizeof(BufAndTag)); If shared_buffers(=NBuffers) is set to something big, this could give some memory churn. And I think it's a bad idea to allocate something this large at checkpoint time, because what happens if that fails? Really not the time you want to discover there's no RAM left. Since you're always going to need this much memory for the system to operate, and the current model has the system running a checkpoint 50% of the time, the only thing that makes sense to me is to allocate it at server start time once and be done with it. That should improve performance over the original patch as well. BufAndTag is a relatively small structure (5 ints). Let's call it 40 bytes; even that's only a 0.5% overhead relative to the shared buffer allocation. If we can speed checkpoints significantly with that much overhead it sounds like a good tradeoff to me. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] MSVC build broken with perl 5.10
Magnus Hagander wrote: I just tried the MSVC build on a system with ActiveState Perl 5.10, and it doesn't work. Some quick debugging before I downgraded to 5.8 showed that this regexp in Project.pm line 262: my $replace_re = qr{^([^:\n\$]+\.c)\s*:\s*(?:%\s*: )?\$(\([^\)]+\))\/(.*)\/[^\/]+$}; matches things properly using Perl 5.8 in for example src/bin/initdb/Makefile (matches a total of around 10 Makefiles), but in 5.10 it simply does not match anything... Any perl guru out there who can comment on why? ;-) The answer is actually simple, the \n needs the multiline modifier, and thus the m needs to be part of the quote-like operator. The perl doc states: This operator quotes (and possibly compiles) its STRING (it seems 5.8 did not compile, but 5.10 does) I feel that it is rather not a perl bug, and that the modifiers need to be put on the qr{}. I do not quite see why this re needs to be multiline in the first place, but I have not touched that in the attached patch, that is ready to apply. (modification works in perl 5.6, 5.8, 5.10) Andreas Project.pm.patch Description: Project.pm.patch -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Terminating a backend
Applied. --- Bruce Momjian wrote: oikBruce Momjian wrote: Bruce Momjian wrote: When we get the termination signal, why can't we just set a global boolean, do a query cancel, and in the setjmp() code block check the global and exit --- at that stage we know we have released all locks and can exit cleanly. I have implemented this idea with the attached patch. One problem I have with my patch is that SIGTERM is currently used by the postmaster to shut down backends. Now because the postmaster knows that all backend are terminating, it can accept a dirtier shutdown than one where we are terminating just one backend and the rest are going to keep running. The new SIGTERM coding is going to exit a backend only in a place where cancel is checked. I have a idea --- to have pg_terminate_backend() set a PGPROC boolean and then send a query cancel signal to the backend --- the backend can then check the boolean and exit if required. I will work on a new version of this patch tomorrow/Monday. Updated patch attached. I didn't modify SIGTERM at all but set a PRPROC boolean and piggybacked on SIGINT. I think I got the PGPROC locking right. I had to split apart pg_signal_backend() so I could do the permission and pid checks independent of the signalling, because I pg_terminate_backend() needs to check, then set the PGPROC variable, then send the signal. I also added an administration doc mention about when to use pg_terminate_backend(). -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] PQmakeResult patch
Here is a patch to add a few functions to libpq: PQmakeResult: creates a new result with attributes but no rows PQsetValue: set a field inside a result, creating a row if necessary (but only one at a time) PQresultAlloc (basically public wrapper to existing internal function). also, the attribute structure (but not the result) is moved to the public header. We thought these functions, particularly PQsetValue, are something positive that came out of the libpqtypes dicussions. merlin Index: interfaces/libpq/exports.txt === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v retrieving revision 1.19 diff -C6 -r1.19 exports.txt *** interfaces/libpq/exports.txt 19 Mar 2008 00:39:33 - 1.19 --- interfaces/libpq/exports.txt 15 Apr 2008 14:22:00 - *** *** 138,143 --- 138,146 PQsendDescribePortal 136 lo_truncate 137 PQconnectionUsedPassword 138 pg_valid_server_encoding_id 139 PQconnectionNeedsPassword 140 lo_import_with_oid 141 + PQmakeResult 142 + PQsetvalue143 + PQresultAlloc 144 Index: interfaces/libpq/fe-exec.c === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v retrieving revision 1.194 diff -C6 -r1.194 fe-exec.c *** interfaces/libpq/fe-exec.c 1 Jan 2008 19:46:00 - 1.194 --- interfaces/libpq/fe-exec.c 15 Apr 2008 14:22:00 - *** *** 60,72 int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! /* * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free --- 60,73 int resultFormat); static void parseInput(PGconn *conn); static bool PQexecStart(PGconn *conn); static PGresult *PQexecFinish(PGconn *conn); static int PQsendDescribe(PGconn *conn, char desc_type, const char *desc_target); ! static int check_field_number(const PGresult *res, ! int field_num); /* * Space management for PGresult. * * Formerly, libpq did a separate malloc() for each field of each tuple * returned by a query. This was remarkably expensive --- malloc/free *** *** 192,203 --- 193,338 result-client_encoding = PG_SQL_ASCII; } return result; } + PGresult * + PQmakeResult(const PGconn *conn, int numAttributes, + PGresAttDesc *attDescs) + { + int i; + PGresult *res; + + if(numAttributes = 0 || !attDescs) + return NULL; + + res = PQmakeEmptyPGresult((PGconn *)conn, PGRES_TUPLES_OK); + if(!res) + return NULL; + + res-attDescs = (PGresAttDesc *) + pqResultAlloc(res, numAttributes * sizeof(PGresAttDesc), TRUE); + + if(!res-attDescs) + { + PQclear(res); + return NULL; + } + + res-numAttributes = numAttributes; + memcpy(res-attDescs, attDescs, numAttributes * sizeof(PGresAttDesc)); + + /* resultalloc the attribute names. */ + res-binary = 1; + for(i=0; i numAttributes; i++) + { + if(attDescs[i].name) + res-attDescs[i].name = pqResultStrdup(res, attDescs[i].name); + else + res-attDescs[i].name = res-null_field; + + if(!res-attDescs[i].name) + { + PQclear(res); + return NULL; + } + + /* Although deprecated, because results can have text+binary columns, + * its easy enough to deduce so set it for completeness. + */ + if(res-attDescs[i].format == 0) + res-binary = 0; + } + + return res; + } + + int + PQsetvalue(PGresult *res, int tup_num, int field_num, + char *value, int len) + { + PGresAttValue *attval; + + if(!check_field_number(res, field_num)) + return FALSE; + + /* Invalid tup_num, must be = ntups */ + if(tup_num res-ntups) + return FALSE; + + /* need to grow the tuple table */ + if(res-ntups = res-tupArrSize) + { + int n = res-tupArrSize ? (res-tupArrSize*3)/2 : 64; + PGresAttValue **tups = (PGresAttValue **) + (res-tuples ? realloc(res-tuples, n*sizeof(PGresAttValue *)) : + malloc(n*sizeof(PGresAttValue *))); + + if(!tups) + return FALSE; + + res-tuples = tups; + res-tupArrSize = n; + } + + /* new to allocate a new tuple */ + if(tup_num == res-ntups !res-tuples[tup_num]) + { + int i; + PGresAttValue *tup = (PGresAttValue *)pqResultAlloc( + res, res-numAttributes * sizeof(PGresAttValue), TRUE); + + if(!tup) + return FALSE; + + /* initialize each column to NULL */ + for(i=0; i res-numAttributes; i++) + { + tup[i].len = NULL_LEN; + tup[i].value = res-null_field; + } + + res-tuples[tup_num] = tup; + res-ntups++; + } + + attval = res-tuples[tup_num][field_num]; + +
Re: [PATCHES] lc_time and localized dates
Euler Taveira de Oliveira wrote: Tom Lane wrote: But as Peter remarks nearby, this discussion is wasted anyway, because there is only one correct answer: whatever Oracle does with these cases is what to_char() should do. My patch does exactly what Oracle does besides one thing: my code does not contain a real capitalization function. It sucks when we have composite names like the portuguese above. I'll post an updated version later. Euler, have you updated this patch yet? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [HACKERS] MSVC build broken with perl 5.10
Zeugswetter Andreas OSB SD wrote: Magnus Hagander wrote: I just tried the MSVC build on a system with ActiveState Perl 5.10, and it doesn't work. Some quick debugging before I downgraded to 5.8 showed that this regexp in Project.pm line 262: my $replace_re = qr{^([^:\n\$]+\.c)\s*:\s*(?:%\s*: )?\$(\([^\)]+\))\/(.*)\/[^\/]+$}; matches things properly using Perl 5.8 in for example src/bin/initdb/Makefile (matches a total of around 10 Makefiles), but in 5.10 it simply does not match anything... Any perl guru out there who can comment on why? ;-) The answer is actually simple, the \n needs the multiline modifier, and thus the m needs to be part of the quote-like operator. The perl doc states: This operator quotes (and possibly compiles) its STRING (it seems 5.8 did not compile, but 5.10 does) I feel that it is rather not a perl bug, and that the modifiers need to be put on the qr{}. I do not quite see why this re needs to be multiline in the first place, but I have not touched that in the attached patch, that is ready to apply. (modification works in perl 5.6, 5.8, 5.10) Thanks, that makes sense. I wonder how it ever worked before. Anyway, patch applied back as far as 8.2. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Partial match in GIN (next vesrion)
http://www.sigaev.ru/misc/partial_match_gin-0.8.gz Reworked interface as it suggested by Gregory (http://archives.postgresql.org/pgsql-patches/2008-04/msg00199.php) and move check of index into expand_indexqual_opclause() as suggested by Heikki (http://archives.postgresql.org/pgsql-patches/2008-04/msg00200.php) http://www.sigaev.ru/misc/tsearch_prefix-0.7.gz Sync with current CVS and partial match GIN patch. Allow full scan index, so now GIN supports search with queries like '!foo'. Implemented via using empty string for prefix search. http://www.sigaev.ru/misc/wildspeed-0.11.tgz Sync with CVS changes and partial match GIN patch. Teach opclass to correct use of recheck feature. -- Teodor Sigaev E-mail: [EMAIL PROTECTED] WWW: http://www.sigaev.ru/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] libpq object hooks patch
Here is an updated version of the object hooks patch. It now supports a list of hooks for a PGconn, and PGresult. This had to re-introduce the concept of hook name. Being that there is now a list, you need a way to reference an item of that list. Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to the conn or result hook list. PQmakeResult must allow the ability to pass a list of object hooks in. So, PQresultObjectHooks was born. pqtypes doesn't need (at least at this time) PQobjectHooks but leaving it out felt unbalanced. Note: PQhookData and PQresultHookData can be removed. There functionality can be reproduced by an API user issuing PQobjectHooks or PQresultObjectHooks and manually looking for there hook (normaly to get at the hook-data). Basically, an API user would do themselves what PQhookData is doing. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ Index: exports.txt === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v retrieving revision 1.19 diff -C6 -r1.19 exports.txt *** exports.txt 19 Mar 2008 00:39:33 - 1.19 --- exports.txt 15 Apr 2008 17:18:38 - *** *** 138,143 --- 138,151 PQsendDescribePortal 136 lo_truncate 137 PQconnectionUsedPassword 138 pg_valid_server_encoding_id 139 PQconnectionNeedsPassword 140 lo_import_with_oid 141 + PQmakeResult 142 + PQsetvalue143 + PQresultAlloc 144 + PQaddObjectHooks 145 + PQhookData146 + PQresultHookData 147 + PQobjectHooks 148 + PQresultObjectHooks 149 \ No newline at end of file Index: fe-connect.c === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v retrieving revision 1.357 diff -C6 -r1.357 fe-connect.c *** fe-connect.c31 Mar 2008 02:43:14 - 1.357 --- fe-connect.c15 Apr 2008 17:18:39 - *** *** 241,253 PQExpBuffer errorMessage); static char *pwdfMatchesString(char *buf, char *token); static char *PasswordFromFile(char *hostname, char *port, char *dbname, char *username); static void default_threadlock(int acquire); - /* global variable because fe-auth.c needs to access it */ pgthreadlock_t pg_g_threadlock = default_threadlock; /* *Connecting to a Database --- 241,252 *** *** 979,990 --- 978,990 * o If your backend wants to use Kerberos authentication then you must *supply both a host name and a host address, otherwise this function *may block on gethostname. * * */ + PostgresPollingStatusType PQconnectPoll(PGconn *conn) { PGresult *res; charsebuf[256]; *** *** 1875,1887 * the connection structure must be freed). */ conn-status = CONNECTION_BAD; return PGRES_POLLING_FAILED; } - /* * makeEmptyPGconn * - create a PGconn data structure with (as yet) no interesting data */ static PGconn * makeEmptyPGconn(void) --- 1875,1886 *** *** 1970,1981 --- 1969,1998 * release data that is to be held for the life of the PGconn structure. * If a value ought to be cleared/freed during PQreset(), do it there not here. */ static void freePGconn(PGconn *conn) { + int i; + + for(i=0; i conn-objHooksCount; i++) + { + if(conn-objHooks[i].connDestroy) + { + conn-objHooks[i].connDestroy((const PGconn *)conn); + free(conn-objHooks[i].name); + } + } + + if(conn-objHooks) + { + free(conn-objHooks); + conn-objHooks = NULL; + conn-objHooksCount = conn-objHooksSize = 0; + } + if (conn-pghost) free(conn-pghost); if (conn-pghostaddr) free(conn-pghostaddr); if (conn-pgport) free(conn-pgport); *** *** 2152,2164 --- 2169,2189 { if (conn) { closePGconn(conn); if (connectDBStart(conn)) + { + int i; + (void) connectDBComplete(conn); + + for(i=0; i conn-objHooksCount; i++) + if(conn-objHooks[i].connReset) + conn-objHooks[i].connReset((const PGconn *)conn); + } } } /* * PQresetStart: *** *** 2176,2198 return connectDBStart(conn); } return 0; } -
Re: [PATCHES] lc_time and localized dates
Bruce Momjian wrote: Euler, have you updated this patch yet? I'm in a process to submit another patch but I want to test on Windows first. -- Euler Taveira de Oliveira http://www.timbira.com/ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] libpq object hooks patch
Andrew Chernow wrote: Here is an updated version of the object hooks patch. It now supports a list of hooks for a PGconn, and PGresult. This had to re-introduce the concept of hook name. Being that there is now a list, you need a way to reference an item of that list. Also added PQobjectHooks and PQresultObjectHooks, to get a pointer to the conn or result hook list. PQmakeResult must allow the ability to pass a list of object hooks in. So, PQresultObjectHooks was born. pqtypes doesn't need (at least at this time) PQobjectHooks but leaving it out felt unbalanced. Note: PQhookData and PQresultHookData can be removed. There functionality can be reproduced by an API user issuing PQobjectHooks or PQresultObjectHooks and manually looking for there hook (normaly to get at the hook-data). Basically, an API user would do themselves what PQhookData is doing. Made some changes: 1. Removed the hookName argument to PQaddObjectHooks, since its in the supplied PQobjectHooks struct. I think the argument was lingering from a previous patch. 2. Added the ability to install global object hooks: PQaddGlobalObjectHooks(PGobjectHooks*). The header docs for this function warns that it should only be used before calling libpq functions or creating application threads. There are no management functions for global hooks, like get or remove. If you add global object hooks, your stuck with them until process death. 3. There is a new internal pqInitObjectHooks(PGconn *) that installs the global object hooks on new conns in the CONNECTION_OK status. I call this function within PQconnectPoll (3 different locations). This will call PQaddObjectHooks(conn) for each global hook (so global hooks are always at the front of a conn's hook list). pqInitObjectHooks checks to see if conn-objHooksCount 0 and if it is, the request is ignored and the function returns success. This only happens during a PQreset and PQresetPoll. // global PQaddGlobalObjectHooks(libpq_debugger_hook); // per-conn PQaddObjectHooks(conn, session_manager_hook); Since the existing list of object hooks is scanned for duplicate names when adding them, you will never run into duplicate object hooks in a connection (or in the global list). If the two examples above were both called by an application, the per-conn call would fail. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ ? exports.list ? libpq.so.5.2 ? object_hooks.patch Index: exports.txt === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v retrieving revision 1.19 diff -C6 -r1.19 exports.txt *** exports.txt 19 Mar 2008 00:39:33 - 1.19 --- exports.txt 16 Apr 2008 01:14:40 - *** *** 138,143 --- 138,152 PQsendDescribePortal 136 lo_truncate 137 PQconnectionUsedPassword 138 pg_valid_server_encoding_id 139 PQconnectionNeedsPassword 140 lo_import_with_oid 141 + PQmakeResult 142 + PQsetvalue143 + PQresultAlloc 144 + PQaddObjectHooks 145 + PQhookData146 + PQresultHookData 147 + PQobjectHooks 148 + PQresultObjectHooks 149 + PQaddGlobalObjectHooks150 \ No newline at end of file Index: fe-connect.c === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v retrieving revision 1.357 diff -C6 -r1.357 fe-connect.c *** fe-connect.c31 Mar 2008 02:43:14 - 1.357 --- fe-connect.c16 Apr 2008 01:14:40 - *** *** 241,253 PQExpBuffer errorMessage); static char *pwdfMatchesString(char *buf, char *token); static char *PasswordFromFile(char *hostname, char *port, char *dbname, char *username); static void default_threadlock(int acquire); - /* global variable because fe-auth.c needs to access it */ pgthreadlock_t pg_g_threadlock = default_threadlock; /* *Connecting to a Database --- 241,252 *** *** 979,990 --- 978,990 * o If your backend wants to use Kerberos authentication then you must *supply both a host name and a host address, otherwise this function *may block on gethostname. * * */ + PostgresPollingStatusType PQconnectPoll(PGconn *conn) { PGresult *res; charsebuf[256]; *** *** 998,1009 --- 998,1010 * We really shouldn't have been polled in these two cases, but we * can handle it. */ case CONNECTION_BAD: return PGRES_POLLING_FAILED; case CONNECTION_OK: +
Re: [PATCHES] Sorting writes during checkpoint
Greg Smith [EMAIL PROTECTED] wrote: On Tue, 15 Apr 2008, ITAGAKI Takahiro wrote: 2x Quad core Xeon, 16GB RAM, 4x HDD (RAID-0) What is the disk controller in this system? I'm specifically curious about what write cache was involved, so I can get a better feel for the hardware your results came from. I used HP ProLiant DL380 G5 with Smart Array P400 with 256MB cache (http://h10010.www1.hp.com/wwpc/us/en/sm/WF06a/15351-15351-3328412-241644-241475-1121516.html) and ext3fs on LVM of CentOS 5.1 (Linux version 2.6.18-53.el5). Dirty region of database was probably larger than disk controller's cache. buf_to_write = (BufAndTag *) palloc(NBuffers * sizeof(BufAndTag)); If shared_buffers(=NBuffers) is set to something big, this could give some memory churn. And I think it's a bad idea to allocate something this large at checkpoint time, because what happens if that fails? Really not the time you want to discover there's no RAM left. Hmm, but I think we need to copy buffer tags into bgwriter's local memory in order to avoid locking taga many times in the sorting. Is it better to allocate sorting buffers at the first time and keep and reuse it from then on? BufAndTag is a relatively small structure (5 ints). Let's call it 40 bytes; even that's only a 0.5% overhead relative to the shared buffer allocation. If we can speed checkpoints significantly with that much overhead it sounds like a good tradeoff to me. I thinks sizeof(BufAndTag) is 20 bytes because sizeof(int) is 4 on typical platforms (and if not, I should rewrite the patch to be always so). It is 0.25% of shared buffers; when shared_buffers is set to 10GB, it takes 25MB of process local memory. If we want to consume less memory for it, RelFileNode in BufferTag could be hashed and packed into an integer; The blockNum order is important for this purpose, but RelFileNode is not. It makes the overhead to 12 bytes per page (0.15%). Is it worth doing? Regards, --- ITAGAKI Takahiro NTT Open Source Software Center -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches