Re: [PATCHES] Libpq COPY optimization patch
Libpq copy speedup patch attached. No input buffer logic involved, just removing the expensive PQconsumeInput call per putCopyData call, and leaving parseInput as is, as discussed before. Alon. libpq_copy.patch Description: Binary data ---(end of broadcast)--- TIP 1: 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
[PATCHES] Adding a --quiet option to initdb
Hi, Attached is a patch which adds --quiet and --q option to initdb. I personally needed this option while writing a document and taking screenshot :) It only shows the error and warning messages, as well as the last lines. I've updated the docs. Regression tests pass. This is my first patch to PostgreSQL source, so please guide me if I have done something wrong. Regards, -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: plPHP, plPerlNG - http://www.commandprompt.com/ Index: doc/src/sgml/ref/initdb.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/initdb.sgml,v retrieving revision 1.35 diff -c -r1.35 initdb.sgml *** doc/src/sgml/ref/initdb.sgml 21 Jun 2005 04:02:31 - 1.35 --- doc/src/sgml/ref/initdb.sgml 25 Jan 2006 13:47:19 - *** *** 235,240 --- 235,252 /listitem /varlistentry + variablelist + varlistentry + termoption--q/option/term + termoption--quiet/option/term + listitem +para + Supresses the messages generated by commandinitdb/command. Only warning + and error messages are displayed. +/para + /listitem + /varlistentry + varlistentry termoption-L replaceable class=parameterdirectory/replaceable/option/term listitem Index: src/bin/initdb/initdb.c === RCS file: /projects/cvsroot/pgsql/src/bin/initdb/initdb.c,v retrieving revision 1.106 diff -c -r1.106 initdb.c *** src/bin/initdb/initdb.c 5 Jan 2006 10:07:46 - 1.106 --- src/bin/initdb/initdb.c 25 Jan 2006 13:47:36 - *** *** 95,100 --- 95,101 static bool debug = false; static bool noclean = false; static bool show_setting = false; + static bool quiet; /* internal vars */ *** *** 1120,1128 test_max_fsm, ok_buffers = 0; ! ! printf(_(selecting default max_connections ... )); ! fflush(stdout); for (i = 0; i connslen; i++) { --- 1121,1131 test_max_fsm, ok_buffers = 0; ! if (!quiet) ! { ! printf(_(selecting default max_connections ... )); ! fflush(stdout); ! } for (i = 0; i connslen; i++) { *** *** 1150,1159 i = connslen - 1; n_connections = trial_conns[i]; ! printf(%d\n, n_connections); ! printf(_(selecting default shared_buffers/max_fsm_pages ... )); ! fflush(stdout); for (i = 0; i bufslen; i++) { --- 1153,1168 i = connslen - 1; n_connections = trial_conns[i]; ! if (!quiet) ! { ! printf(%d\n, n_connections); ! } ! if (!quiet) ! { ! printf(_(selecting default shared_buffers/max_fsm_pages ... )); ! fflush(stdout); ! } for (i = 0; i bufslen; i++) { *** *** 1181,1187 n_buffers = test_buffs; n_fsm_pages = FSM_FOR_BUFS(n_buffers); ! printf(%d/%d\n, n_buffers, n_fsm_pages); } /* --- 1190,1199 n_buffers = test_buffs; n_fsm_pages = FSM_FOR_BUFS(n_buffers); ! if (!quiet) ! { ! printf(%d/%d\n, n_buffers, n_fsm_pages); ! } } /* *** *** 1194,1202 char repltok[100]; char path[MAXPGPATH]; ! fputs(_(creating configuration files ... ), stdout); ! fflush(stdout); ! /* postgresql.conf */ conflines = readfile(conf_file); --- 1206,1216 char repltok[100]; char path[MAXPGPATH]; ! if (!quiet) ! { ! fputs(_(creating configuration files ... ), stdout); ! fflush(stdout); ! } /* postgresql.conf */ conflines = readfile(conf_file); *** *** 1350,1359 char **bki_lines; char headerline[MAXPGPATH]; ! printf(_(creating template1 database in %s/base/1 ... ), pg_data); ! fflush(stdout); ! if (debug) talkargs = -d 5; bki_lines = readfile(bki_file); --- 1364,1376 char **bki_lines; char headerline[MAXPGPATH]; ! if (!quiet) ! { ! printf(_(creating template1 database in %s/base/1 ... ), pg_data); ! fflush(stdout); ! } ! if (debug) talkargs = -d 5; bki_lines = readfile(bki_file); *** *** 1444,1451 NULL }; ! fputs(_(initializing pg_authid ... ), stdout); ! fflush(stdout); snprintf(cmd, sizeof(cmd), \%s\ %s template1 %s, --- 1461,1471 NULL }; ! if (!quiet) ! { ! fputs(_(initializing pg_authid ... ), stdout); ! fflush(stdout); ! } snprintf(cmd, sizeof(cmd), \%s\ %s template1 %s, *** *** 1572,1579 NULL }; ! fputs(_(enabling unlimited row size for system tables ... ), stdout); ! fflush(stdout); snprintf(cmd, sizeof(cmd), \%s\ %s template1 %s, --- 1592,1602 NULL }; ! if (!quiet) ! { ! fputs(_(enabling unlimited row size
Re: [PATCHES] Adding a --quiet option to initdb
On Wed, 2006-01-25 at 16:08 +0200, Devrim GUNDUZ wrote: Hi, Attached is a patch which adds --quiet and --q option to initdb. I personally needed this option while writing a document and taking screenshot :) It only shows the error and warning messages, as well as the last lines. I've updated the docs. Regression tests pass. This is my first patch to PostgreSQL source, so please guide me if I have done something wrong. Devrim, What's wrong with just sending stdout to /dev/null? If that eats error messages too then we should probably fix initdb to send those to stderr. But if we are going to do this, then I also noticed a couple of things: . you should explicitly initialize the quiet variable, in keeping with the style of the others nearby. . the idiom if (! quiet) { fputs(_(some message),stdout); fflush(stdout); } should not be endlessly repeated. Make it a macro or a function. I wonder if we can just set rid of all those fflush() calls by unbuffering stdout with a single call to setbuf() or setvbuf()? cheers andrew ---(end of broadcast)--- TIP 1: 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: [HACKERS] [PATCHES] Adding a --quiet option to initdb
Hi Andrew, On Wed, 2006-01-25 at 09:28 -0500, Andrew Dunstan wrote: What's wrong with just sending stdout to /dev/null? If that eats error messages too then we should probably fix initdb to send those to stderr. We have the same options with reindexdb, for example. I think a command line option But if we are going to do this, then I also noticed a couple of things: . you should explicitly initialize the quiet variable, in keeping with the style of the others nearby. Oh, I've missed it. Patch updated. . the idiom if (! quiet) { fputs(_(some message),stdout); fflush(stdout); } should not be endlessly repeated. Make it a macro or a function. I'm looking at it now. Thanks for your comments. I'll provide a new patch soon, which will also include Alvaro's suggestion about surpression both header and footer (--make-initdb-really-quiet-mode :) ) Regards, -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: plPHP, plPerlNG - http://www.commandprompt.com/ ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Adding a --quiet option to initdb
Hi, On Wed, 2006-01-25 at 17:22 +0200, Devrim GUNDUZ wrote: On Wed, 2006-01-25 at 09:28 -0500, Andrew Dunstan wrote: What's wrong with just sending stdout to /dev/null? If that eats error messages too then we should probably fix initdb to send those to stderr. We have the same options with reindexdb, for example. I think a command line option (Opps...) We have the same options with reindexdb, for example. I think a command line options is better and required. Regards, -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: plPHP, plPerlNG - http://www.commandprompt.com/ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Adding a --quiet option to initdb
Devrim GUNDUZ wrote: We have the same options with reindexdb, for example. reindexdb and friends inherited that option from psql. On a green field, they probably wouldn't have it. psql has more complex semantics, so it's not clear whether that's the same thing. I think a command line options is better and required. I think we need more proof of that than a use case involving taking screen shots. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Adding a --quiet option to initdb
Devrim GUNDUZ [EMAIL PROTECTED] writes: Attached is a patch which adds --quiet and --q option to initdb. Why is this a good idea? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Adding a --quiet option to initdb
Hi, On Wed, 2006-01-25 at 11:28 -0500, Tom Lane wrote: Devrim GUNDUZ [EMAIL PROTECTED] writes: Attached is a patch which adds --quiet and --q option to initdb. Why is this a good idea? I was playing with 8.2 RPM init script and thought that instead of directing the output to /dev/null, it would be better to use a command line option for that. Also, we are designing a new installer project and --quiet might help us. I would rise this idea in -hackers before providing a patch, but since this is my first patch, I thought it would be a good exercise for me. Regards, -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: plPHP, plPerlNG - http://www.commandprompt.com/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Adding a --quiet option to initdb
Peter Eisentraut wrote: Devrim GUNDUZ wrote: We have the same options with reindexdb, for example. reindexdb and friends inherited that option from psql. On a green field, they probably wouldn't have it. psql has more complex semantics, so it's not clear whether that's the same thing. I think a command line options is better and required. I think we need more proof of that than a use case involving taking screen shots. Agreed on both points. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (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: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Adding a --quiet option to initdb
Hi, On Wed, 2006-01-25 at 16:32 +0100, Peter Eisentraut wrote: I think a command line options is better and required. I think we need more proof of that than a use case involving taking screen shots. I've just explained my points as a reply to Tom's mail. Regards, -- The PostgreSQL Company - Command Prompt, Inc. 1.503.667.4564 PostgreSQL Replication, Consulting, Custom Development, 24x7 support Managed Services, Shared and Dedicated Hosting Co-Authors: plPHP, plPerlNG - http://www.commandprompt.com/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Adding a --quiet option to initdb
Devrim GUNDUZ wrote: Hi, On Wed, 2006-01-25 at 11:28 -0500, Tom Lane wrote: Devrim GUNDUZ [EMAIL PROTECTED] writes: Attached is a patch which adds --quiet and --q option to initdb. Why is this a good idea? I was playing with 8.2 RPM init script and thought that instead of directing the output to /dev/null, it would be better to use a command line option for that. Also, we are designing a new installer project and --quiet might help us. I would rise this idea in -hackers before providing a patch, but since this is my first patch, I thought it would be a good exercise for me. OK, as long as you understand that the patch should not be applied. It might be valuable from only one person is not enough. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (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 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [GENERAL] Different exponent in error messages
Applied. --- Bruce Momjian wrote: Tom Lane wrote: Michael Fuhr [EMAIL PROTECTED] writes: I think this is what Andrus is seeing: test= CREATE TABLE foo (n numeric(9,3)); CREATE TABLE test= INSERT INTO foo VALUES (100); ERROR: numeric field overflow DETAIL: The absolute value is greater than or equal to 10^6 for field with precision 9, scale 3. test= INSERT INTO foo VALUES (10); ERROR: numeric field overflow DETAIL: The absolute value is greater than or equal to 10^9 for field with precision 9, scale 3. Hm, I thought I tested that same case, but I must've messed up somehow. Anyway, the code seems to be intentionally reporting the log10 of the actual input value, not the limiting log10 for the field size. This behavior goes at least as far back as PG 7.0, so I'm disinclined to change it. We could talk about altering the message wording though, if you have a suggestion for something you'd find less confusing. Pre-7.4 versions say ERROR: overflow on numeric ABS(value) = 10^9 for field with precision 9 scale 3 so it looks like we just fixed the grammar during the 7.4 message wording cleanup, without reflecting about whether the meaning was clear. Yes, this message clearly needs help. Here is what I developed: test= CREATE TABLE foo (n numeric(9,3)); CREATE TABLE test= INSERT INTO foo VALUES (1000); ERROR: numeric field overflow DETAIL: A field with precision 9, scale 3 must have an absolute value less than 10^6. and the 10^6 is based on the max digits to the left of the decimal point, not the input value. -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 Index: src/backend/utils/adt/numeric.c === RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v retrieving revision 1.88 diff -c -c -r1.88 numeric.c *** src/backend/utils/adt/numeric.c 22 Nov 2005 18:17:23 - 1.88 --- src/backend/utils/adt/numeric.c 23 Jan 2006 20:00:54 - *** *** 3206,3213 ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg(numeric field overflow), ! errdetail(The absolute value is greater than or equal to 10^%d for field with precision %d, scale %d., ! ddigits - 1, precision, scale))); break; } ddigits -= DEC_DIGITS; --- 3206,3213 ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg(numeric field overflow), ! errdetail(A field with precision %d, scale %d must have an absolute value less than 10^%d., ! precision, scale, maxdigits))); break; } ddigits -= DEC_DIGITS; ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings -- Bruce Momjian| http://candle.pha.pa.us pgman@candle.pha.pa.us | (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: 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] [HACKERS] Adding a --quiet option to initdb
Bruce Momjian pgman@candle.pha.pa.us writes: Devrim GUNDUZ wrote: Hi, On Wed, 2006-01-25 at 11:28 -0500, Tom Lane wrote: Devrim GUNDUZ [EMAIL PROTECTED] writes: Attached is a patch which adds --quiet and --q option to initdb. Why is this a good idea? I was playing with 8.2 RPM init script and thought that instead of directing the output to /dev/null, it would be better to use a command line option for that. Also, we are designing a new installer project and --quiet might help us. OK, as long as you understand that the patch should not be applied. It might be valuable from only one person is not enough. I always wondered why the Redhat init scripts thought it was a clever idea to redirect the output to /dev/null. It seems like a pessimal user interface choice. Every time I have to work with a Redhat machine where Postgres isn't starting up the first thing I have to do is edit the init script so I can what the problem is. -- greg ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [HACKERS] Adding a --quiet option to initdb
Greg Stark [EMAIL PROTECTED] writes: I always wondered why the Redhat init scripts thought it was a clever idea to redirect the output to /dev/null. It seems like a pessimal user interface choice. Every time I have to work with a Redhat machine where Postgres isn't starting up the first thing I have to do is edit the init script so I can what the problem is. Yeah, that's finally fixed in the latest versions. The problem was that sending the postmaster log into a file wasn't a good long-term idea because of the lack of any way to rotate the log. Current RPMs set up redirect_stderr with some reasonable rotation options instead. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] pgbench: Support Multiple Simultaneous Runs (with Mean and Std. Dev.)
I guess I was thinking that the multiple iterations allow for easier pinpointing of noise on the system. I think I remember originally writing this during an episode where successive runs of pgbench were yielding wildly different results, and I wound up wanting an easy way to detect this and examine it further, so standard deviations in smaller sets of runs seemed an easy way to do it. With large numbers of transactions, any noise in the system would get smoothed out. I guess I like the ability to see the incremental behavior of the system when using pgbench, and this lets me run it for a long enough duration inspecting more often. If this isn't deemed generally useful functionality, that's fine. -- Thomas F. O'Connell Database Architecture and Programming Co-Founder Sitening, LLC http://www.sitening.com/ 110 30th Avenue North, Suite 6 Nashville, TN 37203-6320 615-260-0005 (cell) 615-469-5150 (office) 615-469-5151 (fax) ---(end of broadcast)--- TIP 1: 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] Libpq COPY optimization patch
Alon Goldshuv [EMAIL PROTECTED] writes: Libpq copy speedup patch attached. No input buffer logic involved, just removing the expensive PQconsumeInput call per putCopyData call, and leaving parseInput as is, as discussed before. Applied (with a better comment) to HEAD and 8.1 branches. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] External Sort performance patch
The enclosed patch substantially improves large sort performance, in the general case cvstip: elapsed 5693 sec, CPU 196 sec patch: elapsed 4132 sec, CPU 90 sec The patch implements dynamically increasing number of logical tapes when sufficient memory is available to make that efficient. cvstip runs with a static number of tapes (7) whereas the patch was able to allocate 104 tapes to the task. This has the effect of almost completely removing intermediate merge runs and hence the increased performance. From Jeffrey W. Baker's original idea http://archives.postgresql.org/pgsql-performance/2005-09/msg00430.php and followup comments http://archives.postgresql.org/pgsql-hackers/2005-10/msg00015.php It is expected this will substantially improve performance for large ORDER BY, GROUP BY and CREATE INDEX statements. The guesstimated default setting of the OPTIMAL_MERGE_BUFFER_SIZE of 262144 means that the default setting of work_mem will still use only 7 tapes, though setting work_mem 2MB will yield improvements. Further testing and/or patch comments are requested. All changes are isolated to src/backend/utils/sort/tuplesort.c Patch applies cleanly and passes make check on cvstip (though this code path is not tested in the regression tests anyway). Test details: Run the following sort on my laptop (512MB RAM) postgres=# set work_mem=65536; SET Time: 0.801 ms postgres=# select * from d order by 1,2 limit 1; col1 | col2 --+- 1 | ees (1 row) Time: 4133122.769 ms postgres=# \d d Table public.d Column | Type | Modifiers +-+--- col1 | integer | col2 | text| postgres=# select count(*) from d; count --- 1 (1 row) Time: 248283.128 ms postgres=# select pg_relation_size('d'); pg_relation_size -- 6450397184 (1 row) Time: 98.629 ms postgres=# trace_sort was enabled for both runs and these are attached as files to this mail. Test data was anti-sorted, but the ordering of data is not relevant to the algorithm anyway, except when the data is already almost perfectly sorted, in which case there is typically only one run anyway. Best Regards, Simon Riggs LOG: begin tuple sort: nkeys = 2, workMem = 65536, randomAccess = t LOG: switching to external sort: CPU 0.25s/0.39u sec elapsed 1.18 sec LOG: finished writing run 1: CPU 0.79s/15.29u sec elapsed 19.75 sec LOG: finished writing run 2: CPU 1.29s/25.13u sec elapsed 33.81 sec LOG: finished writing run 3: CPU 1.75s/33.26u sec elapsed 45.11 sec LOG: finished writing run 4: CPU 2.42s/41.00u sec elapsed 58.49 sec LOG: finished writing run 5: CPU 3.04s/48.57u sec elapsed 70.06 sec LOG: finished writing run 6: CPU 3.67s/55.95u sec elapsed 81.97 sec LOG: finished writing run 7: CPU 4.31s/63.22u sec elapsed 94.80 sec LOG: finished writing run 8: CPU 4.89s/70.54u sec elapsed 106.45 sec LOG: finished writing run 9: CPU 5.55s/77.76u sec elapsed 119.85 sec LOG: finished writing run 10: CPU 6.19s/85.27u sec elapsed 132.30 sec LOG: finished writing run 11: CPU 6.81s/92.68u sec elapsed 145.78 sec LOG: finished writing run 12: CPU 7.47s/99.63u sec elapsed 159.01 sec LOG: finished writing run 13: CPU 8.11s/106.92u sec elapsed 170.88 sec LOG: finished writing run 14: CPU 8.76s/113.67u sec elapsed 184.52 sec LOG: finished writing run 15: CPU 9.39s/120.32u sec elapsed 196.05 sec LOG: finished writing run 16: CPU 10.04s/127.61u sec elapsed 211.77 sec LOG: finished writing run 17: CPU 10.67s/134.80u sec elapsed 224.49 sec LOG: finished writing run 18: CPU 11.27s/141.30u sec elapsed 234.93 sec LOG: finished writing run 19: CPU 11.82s/147.79u sec elapsed 245.44 sec LOG: finished writing run 20: CPU 12.40s/154.45u sec elapsed 256.55 sec LOG: finished writing run 21: CPU 13.05s/160.99u sec elapsed 268.55 sec LOG: finished writing run 22: CPU 13.69s/168.26u sec elapsed 280.16 sec LOG: finished writing run 23: CPU 14.29s/175.05u sec elapsed 291.57 sec LOG: finished writing run 24: CPU 14.95s/182.21u sec elapsed 304.59 sec LOG: finished writing run 25: CPU 15.52s/188.82u sec elapsed 315.75 sec LOG: finished writing run 26: CPU 16.23s/196.91u sec elapsed 329.30 sec LOG: finished writing run 27: CPU 16.82s/203.89u sec elapsed 340.90 sec LOG: finished writing run 28: CPU 17.48s/210.93u sec elapsed 354.44 sec LOG: finished writing run 29: CPU 18.07s/217.16u sec elapsed 365.60 sec LOG: finished writing run 30: CPU 18.69s/223.92u sec elapsed 377.15 sec LOG: finished writing run 31: CPU 19.39s/231.10u sec elapsed 391.09 sec LOG: finished writing run 32: CPU 20.03s/237.83u sec elapsed 404.30 sec LOG: finished writing run 33: CPU 20.67s/245.04u sec elapsed 417.78 sec LOG: finished writing run 34: CPU 21.35s/253.22u sec elapsed 431.70 sec LOG: finished writing run 35: CPU 22.02s/259.88u sec elapsed 444.64 sec LOG: finished writing run 36: CPU 22.63s/266.52u sec elapsed 455.38 sec LOG: finished writing run 37: CPU
Re: [PATCHES] [HACKERS] CIDR/INET improvements
Joachim Wieland [EMAIL PROTECTED] writes: FWIW, I append the patch I've done a few weeks ago. It adds an inettocidr cast function. I've incorporated this code into the INET cleanup patch just committed. Thanks! regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[PATCHES] plperl: return_next memory leak
Per a bug report from Theo Schlossnagle, plperl_return_next() leaks memory in the executor's per-query memory context. It also inefficient: it invokes get_call_result_type() and TupleDescGetAttInMetadata() for every call to return_next, rather than invoking them once (per PL/Perl function call) and memoizing the result. This patch makes the following changes: - refactor the code to include all the per PL/Perl function call data inside a single struct, current_call_data. This means we don't need to save and restore N pointers for every recursive call into PL/Perl, we can just save and restore one. - lookup the return type metadata needed by plperl_return_next() once, and then stash it in current_call_data, so avoid doing the lookup for every call to return_next. - create a temporary memory context in which to evaluate the return type's input functions. This memory context is reset for each call to return_next. The patch appears to fix the memory leak, and substantially improves the performance of return_next (~140 ms vs. ~90 ms, in my simple trivial test -- perhaps less of an improvement for more realistic functions). The patch isn't minimally invasive -- I was thinking about developing a more limited patch for REL8_1_STABLE, but I'm not sure if it is worth the trouble. Barring any objections, I'll apply this patch to HEAD and possibly REL8_1_STABLE tomorrow. -Neil *** src/pl/plperl/plperl.c a4ba9c99e0d5d19ee7fcf9fb6762594a43fd2fae --- src/pl/plperl/plperl.c 9353d541fb99b966b150480e5ebe390bd155f2e5 *** *** 84,94 SV *reference; } plperl_proc_desc; /** * Global data **/ ! static int plperl_firstcall = 1; static bool plperl_safe_init_done = false; static PerlInterpreter *plperl_interp = NULL; static HV *plperl_proc_hash = NULL; --- 84,108 SV *reference; } plperl_proc_desc; + /* + * The information we cache for the duration of a single call to a + * function. + */ + typedef struct plperl_call_data + { + plperl_proc_desc *prodesc; + FunctionCallInfo fcinfo; + Tuplestorestate *tuple_store; + TupleDesc ret_tdesc; + AttInMetadata *attinmeta; + MemoryContext tmp_cxt; + } plperl_call_data; + /** * Global data **/ ! static bool plperl_firstcall = true; static bool plperl_safe_init_done = false; static PerlInterpreter *plperl_interp = NULL; static HV *plperl_proc_hash = NULL; *** *** 95,105 static bool plperl_use_strict = false; ! /* these are saved and restored by plperl_call_handler */ ! static plperl_proc_desc *plperl_current_prodesc = NULL; ! static FunctionCallInfo plperl_current_caller_info; ! static Tuplestorestate *plperl_current_tuple_store; ! static TupleDesc plperl_current_tuple_desc; /** * Forward declarations --- 109,116 static bool plperl_use_strict = false; ! /* this is saved and restored by plperl_call_handler */ ! static plperl_call_data *current_call_data = NULL; /** * Forward declarations *** *** 157,163 EmitWarningsOnPlaceholders(plperl); plperl_init_interp(); ! plperl_firstcall = 0; } --- 168,174 EmitWarningsOnPlaceholders(plperl); plperl_init_interp(); ! plperl_firstcall = false; } *** *** 292,298 plperl_safe_init_done = true; } - /* * Perl likes to put a newline after its error messages; clean up such */ --- 303,308 *** *** 565,574 plperl_call_handler(PG_FUNCTION_ARGS) { Datum retval; ! plperl_proc_desc *save_prodesc; ! FunctionCallInfo save_caller_info; ! Tuplestorestate *save_tuple_store; ! TupleDesc save_tuple_desc; plperl_init_all(); --- 575,581 plperl_call_handler(PG_FUNCTION_ARGS) { Datum retval; ! plperl_call_data *save_call_data; plperl_init_all(); *** *** 572,582 plperl_init_all(); ! save_prodesc = plperl_current_prodesc; ! save_caller_info = plperl_current_caller_info; ! save_tuple_store = plperl_current_tuple_store; ! save_tuple_desc = plperl_current_tuple_desc; ! PG_TRY(); { if (CALLED_AS_TRIGGER(fcinfo)) --- 579,585 plperl_init_all(); ! save_call_data = current_call_data; PG_TRY(); { if (CALLED_AS_TRIGGER(fcinfo)) *** *** 586,595 } PG_CATCH(); { ! plperl_current_prodesc = save_prodesc; ! plperl_current_caller_info = save_caller_info; ! plperl_current_tuple_store = save_tuple_store; ! plperl_current_tuple_desc =
Re: [PATCHES] Fix overflow of bgwriter's request queue
Tom Lane [EMAIL PROTECTED] wrote: ITAGAKI Takahiro [EMAIL PROTECTED] writes: Attached is a revised patch. It became very simple, but I worry that one magic number (BUFFERS_PER_ABSORB) is still left. Have you checked that this version of the patch fixes the problem you saw originally? Does the problem come back if you change BUFFERS_PER_ABSORB to too large a value? The problem on my machine was resolved by this patch. I tested it and logged the maximum of BgWriterShmem-num_requests for each checkpoint. Test condition was: - shared_buffers = 65536 - connections = 30 The average of maximums was 25857 and the while max was 31807. They didn't exceed the max_requests(= 65536). I suspect it'd probably be sufficient to absorb requests every few times through the fsync loop, too, if you want to experiment with that. In the above test, smgrsync took 50 sec for syncing 32 files. This means absorb are requested every 1.5 sec, which is less frequent than absorbs by normal activity of bgwriter (bgwriter_delay=200ms). So I assume absorb requests the fsync loop would not be a problem. BUFFERS_PER_ABSORB = 10 (absorb per 1/10 of shared_buffers) is enough at least on my machine, but it doesn't necessarily work well in all environments. If we need to set BUFFERS_PER_ABSORB to a reasonably value, I think the number of active backends might be useful; for example, half of num of backends. --- ITAGAKI Takahiro NTT Cyber Space Laboratories ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org