Re: [PATCHES] [HACKERS] WAL: O_DIRECT and multipage-writer (+
On Tue, 2005-03-01 at 13:53 -0800, Mark Wong wrote: On Thu, Feb 03, 2005 at 07:25:55PM +0900, ITAGAKI Takahiro wrote: Hello everyone. I fixed two bugs in the patch that I sent before. Check and test new one, please. Ok, finally got back into the office and was able to run 1 set of tests. So the new baseline result with 8.0.1: http://www.osdl.org/projects/dbt2dev/results/dev4-010/309/ Throughput: 3639.97 Results with the patch but open_direct not set: http://www.osdl.org/projects/dbt2dev/results/dev4-010/308/ Throughput: 3494.72 Results with the patch and open_direct set: http://www.osdl.org/projects/dbt2dev/results/dev4-010/312/ Throughput: 3489.69 You can verify that the wall_sync_method is set to open_direct under the database parameters link, but I'm wondering if I missed something. It looks a little odd the the performance dropped. Is there anything more to say on this? Is it case-closed, or is there further work underway - I can't see any further chat on this thread. These results show it doesn't work better on larger systems. The original testing showed it worked better on smaller systems - is there still scope to include this for smaller configs? If not, thanks for taking the time to write the patch and investigate whether changes in this area would help. Not every performance patch improves things, but that doesn't mean we shouldn't try... Best Regards, Simon Riggs ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
[PATCHES] pgcrypto: openssl digest fix
Some builds (depends on crypto engine support?) of OpenSSL 0.9.7x have EVP_DigestFinal function which which clears all of EVP_MD_CTX. This makes pgcrypto crash in functions which re-use one digest context several times: hmac() and crypt() with md5 algorithm. Following patch fixes it by carring the digest info around EVP_DigestFinal and re-initializing cipher. Please apply this also to stable branches (8.0 / 7.4). Note that this can be blamed on OpenSSL 0.9.7x backwards- compatibility functions: 0.9.6x and new 0.9.7x API (EVP_DigestFinal_ex) do clear the secret data but keep the general algorithm info. But still, the fact is that pgcrypto was relying on undocumented beheviour. -- marko Index: contrib/pgcrypto/openssl.c === RCS file: /opt/cvs2/pgsql/contrib/pgcrypto/openssl.c,v retrieving revision 1.13 diff -u -c -r1.13 openssl.c *** contrib/pgcrypto/openssl.c 29 Nov 2003 22:39:28 - 1.13 --- contrib/pgcrypto/openssl.c 11 Mar 2005 15:39:34 - *** *** 73,80 --- 73,87 digest_finish(PX_MD * h, uint8 *dst) { EVP_MD_CTX *ctx = (EVP_MD_CTX *) h-p.ptr; + const EVP_MD *md = EVP_MD_CTX_md(ctx); EVP_DigestFinal(ctx, dst, NULL); + + /* +* Some builds of 0.9.7x clear all of ctx in EVP_DigestFinal. +* Fix it by reinitializing ctx. +*/ + EVP_DigestInit(ctx, md); } static void ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] Add fprintf macro
I have applied the following patch: Add fprintf() custom version to libpgport. Document use of macros for pg_printf functions. Bump major versions of all interfaces to handle movement of get_progname from libpq to libpgport in 8.0, and probably other libpgport changes in 8.1. -- 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/bootstrap/bootscanner.l === RCS file: /cvsroot/pgsql/src/backend/bootstrap/bootscanner.l,v retrieving revision 1.38 diff -c -c -r1.38 bootscanner.l *** src/backend/bootstrap/bootscanner.l 31 Dec 2004 21:59:34 - 1.38 --- src/backend/bootstrap/bootscanner.l 11 Mar 2005 19:02:15 - *** *** 42,47 --- 42,48 /* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */ + #undef fprintf #define fprintf(file, fmt, msg) ereport(ERROR, (errmsg_internal(%s, msg))) Index: src/backend/parser/scan.l === RCS file: /cvsroot/pgsql/src/backend/parser/scan.l,v retrieving revision 1.120 diff -c -c -r1.120 scan.l *** src/backend/parser/scan.l 22 Feb 2005 04:36:22 - 1.120 --- src/backend/parser/scan.l 11 Mar 2005 19:02:16 - *** *** 28,33 --- 28,34 /* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */ + #undef fprintf #define fprintf(file, fmt, msg) ereport(ERROR, (errmsg_internal(%s, msg))) extern YYSTYPE yylval; Index: src/backend/utils/misc/guc-file.l === RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v retrieving revision 1.29 diff -c -c -r1.29 guc-file.l *** src/backend/utils/misc/guc-file.l 1 Jan 2005 05:43:08 - 1.29 --- src/backend/utils/misc/guc-file.l 11 Mar 2005 19:02:16 - *** *** 19,24 --- 19,25 #include utils/guc.h /* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */ + #undef fprintf #define fprintf(file, fmt, msg) ereport(ERROR, (errmsg_internal(%s, msg))) static unsigned ConfigFileLineno; Index: src/include/port.h === RCS file: /cvsroot/pgsql/src/include/port.h,v retrieving revision 1.71 diff -c -c -r1.71 port.h *** src/include/port.h 11 Mar 2005 17:20:34 - 1.71 --- src/include/port.h 11 Mar 2005 19:02:17 - *** *** 112,128 extern int pg_snprintf(char *str, size_t count, const char *fmt,...) /* This extension allows gcc to check the format string */ __attribute__((format(printf, 3, 4))); extern int pg_printf(const char *fmt,...) /* This extension allows gcc to check the format string */ __attribute__((format(printf, 1, 2))); #ifdef __GNUC__ ! #define vsnprintf(...)pg_vsnprintf(__VA_ARGS__) #define snprintf(...) pg_snprintf(__VA_ARGS__) #define printf(...) pg_printf(__VA_ARGS__) #else #define vsnprintf pg_vsnprintf #define snprintf pg_snprintf #define printfpg_printf #endif #endif --- 112,138 extern int pg_snprintf(char *str, size_t count, const char *fmt,...) /* This extension allows gcc to check the format string */ __attribute__((format(printf, 3, 4))); + extern int pg_fprintf(FILE *stream, const char *fmt,...) + /* This extension allows gcc to check the format string */ + __attribute__((format(printf, 2, 3))); extern int pg_printf(const char *fmt,...) /* This extension allows gcc to check the format string */ __attribute__((format(printf, 1, 2))); + /* + *The GCC-specific code below prevents the __attribute__(... 'printf') + *above from being replaced, and this is required because gcc doesn't + *know anything about pg_printf. + */ #ifdef __GNUC__ ! #define vsnprintf(...) pg_vsnprintf(__VA_ARGS__) #define snprintf(...) pg_snprintf(__VA_ARGS__) + #define fprintf(...) pg_fprintf(__VA_ARGS__) #define printf(...) pg_printf(__VA_ARGS__) #else #define vsnprintf pg_vsnprintf #define snprintf pg_snprintf + #define fprintf pg_fprintf #define printfpg_printf #endif #endif Index: src/interfaces/ecpg/compatlib/Makefile === RCS file: /cvsroot/pgsql/src/interfaces/ecpg/compatlib/Makefile,v retrieving revision 1.19 diff -c -c -r1.19 Makefile *** src/interfaces/ecpg/compatlib/Makefile 18 Jan 2005 05:00:15 - 1.19 --- src/interfaces/ecpg/compatlib/Makefile 11 Mar 2005 19:02:17 -
Re: [PATCHES] [pgsql-hackers-win32] Repleacement for src/port/snprintf.c
I have reviewed this patch, and I already added these changes myself in CVS. Thanks. --- Nicolai Tufar wrote: On Mon, Feb 21, 2005 at 10:53:08PM -0500, Bruce Momjian wrote: Applied. Thanks a lot. The patch attached solves the tread safety problem. Please review it before applying, I am not sure I am doing the right thing On Tue, 22 Feb 2005 19:57:15 +0100, Kurt Roeckx [EMAIL PROTECTED] wrote: The configure test is a little broken. It needs to quote the $'s. I've rewritten the test a little. This one needs applying too. $'s do get scrambled. Best regards, Nicolai. [ Attachment, skipping... ] ---(end of broadcast)--- TIP 7: 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 7: don't forget to increase your free space map settings
Re: [PATCHES] [INTERFACES] bcc32.mak for libpq broken? (distro 8.0.0) (fwd)
Bruce Momjian pgman@candle.pha.pa.us rta: Bernyi Gbor wrote: The problem is something else. Gives the same message. Bruce Momjian pgman@candle.pha.pa.us ?rta: Ber?nyi G?bor wrote: Dear Bruce, Didn't work: Fatal bcc32.mak 169: No terminator specified for in-line file operator Look forward to hear of you again, Gabor OK, new bcc32.mak attached. It turns out the actions were indented with spaces instead of tabs. OK, I think I figured it out. Resource files are done differently in MS make and bcc make. I have attached a new file. Please test and let me know. If you can keep testing, we will eventually get this working. Thanks. -- 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 Hello Bruce, The output: 29 rows, each consists of the single message file not found. Should I send you a bcc makefile that works? have one for my project that the IDE created automatically - if you think it helps to understand the syntax. Keep trying, Gabor ---(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] [BUGS] BUG #1466: #maintenace_work_mem = 16384
Magnus Hagander [EMAIL PROTECTED] writes: Here is an updated patch, that should take care of this. Tested that it solves the problem reported. I compared this to the version Bruce applied earlier and decided that his version was good. I don't think we should change the original logic that treated write_syslogger_file as an independent special destination for the syslogger process only. I've backpatched that version to 8.0 branch. If the logger is complaining, it's quite possibly because it's unable to write to its file. Now that you mention it, doesn't this code go into infinite recursion if write_syslogger_file_binary() tries to ereport? I haven't looked at this part, it appears a separate (but closely related) issue. Actually, your change to make write_syslogger_file_binary() use write_stderr seems like a fine solution to this problem. However you didn't get it quite right: the call has to be more like /* can't use ereport here because of possible recursion */ if (rc != count) write_stderr(could not write to log file: %s\n, strerror(errno)); since write_stderr doesn't know about %m and doesn't supply a free newline. Applied and backpatched to 8.0. 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] BUG #1466: syslogger issues
Patch applied by Tom. Thanks. Backpatched to 8.0.X. --- Magnus Hagander wrote: There is special code in the send_message_to_server_log function to make sure it's written directly to the file. If the logger is complaining, it's quite possibly because it's unable to write to its file. Now that you mention it, doesn't this code go into infinite recursion if write_syslogger_file_binary() tries to ereport? Yes, apparently. Actually, elog.c code should look like this: if ((Log_destination LOG_DESTINATION_STDERR) ...) { if (am_syslogger) write_syslogger_file(buf.data, buf.len); else fwrite(buf.data, 1, buf.len, stderr); } This avoids unnecessary pipe traffic (which might fail too) and gettext translation. That's sort of what I thought, but without being certain at all. Next, the elog call in write_syslogger_file_binary will almost certainly loop, so it should call write_stderr then (since eventlog is usually fixed-size with cyclic writing, even in out-of-disk-space conditions something might get logged). Ok. I've included these changes in the attached patch. Haven't tested those specific codepaths, but the other changes still work... 3rd, I've been proposing to have redirect_stderr=true on by default at least on win32 earlier, I still think this is reasonable. It's already the default if you install from the MSI installer. //Magnus Content-Description: stderr.patch [ Attachment, skipping... ] ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org -- 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: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] A way to let Vacuum warn if FSM settings are low. [final?]
I have applied your patch with minor modifications. Applied version attached. I think the pages message: INFO: free space map: 44 relations, 28 pages stored; 704 total pages used DETAIL: FSM size: 1000 relations + 2 pages = 182 kB shared memory. should remain DEBUG2 for non-VERBOSE, and INFO for VERBOSE. The information is pretty complex and probably of little interest to a typical vacuum user. In fact, the new messages make the information even less important because problems are now flagged. I adjusted your output levels for the new messages. I realize the checkpoint warning is a LOG message, but it has to be because there is no active session when a checkpoint is being run. In the case of VACUUM, there is an active session so I think the messages should be sent to that session. Sending them to the logs and not to the user seems unusual because they are the ones who asked for the VACUUM. I realize they might not be able to change the server settings. These new messages: NOTICE: max_fsm_relations(1000) equals the number of relations checked HINT: You have = 44 relations. Consider increasing the configuration parameter max_fsm_relations. NOTICE: the number of page slots needed (704) exceeds max_fsm_pages (2) HINT: Consider increasing the configuration parameter max_fsm_relations to a value over 704. VACUUM should be NOTICE. NOTICE is for unusual events that are not warnings, and that fits these cases. If the administrator wants those in the logs, he can set log_min_messages to NOTICE. I also adjusted your output strings to more closely match our checkpoint warning message. Another idea would be to send the output to both the client and the logs by default. --- Ron Mayer wrote: On Sun, 27 Feb 2005, Simon Riggs wrote: On Fri, 2005-02-25 at 16:48 -0800, Ron Mayer wrote: Getting closer? For me, yes. [...] The not-warnings seem a little wordy for me, but they happen when and how I would hope for. So, for me, it looks like a polish of final wording and commit. Thanks for the feedback. How about I replace the grammatically poor: LOG: max_fsm_relations(%d) is equal than the number of relations vacuum checked (%d), HINT: You probably have more than %d relations. You should increase max_fsm_relations. Pages needed for max_fsm_pages may have been underestimated. with this: LOG: max_fsm_relations(100) equals the number of relations checked HINT: You have = 100 relations. You should increase max_fsm_relations. and replace this: LOG: max_fsm_pages(%d) is smaller than the actual number of page slots needed(%.0f), HINT: You may want to increase max_fsm_pages to be larger than %.0f with the slightly smaller LOG: the number of page slots needed (2832) exceeds max_fsm_pages (1601) HINT: You may want to increase max_fsm_pages to a value over 2832. These updated messages would fit on an 80-column display if the numbers aren't too big. Here's 80 characters for a quick reference. 01234567890123456789012345678901234567890123456789012345678901234567890123456789 The pages needed...underestimate in the first message was no longer useful anyway; since it's no longer logging fsm_pages stuff when the max_fsm_relations condition occurred anyway Ron The patch now looks like: % diff -u postgresql-8.0.1/src/backend/storage/freespace/freespace.c postgresql-patched/src/backend/storage/freespace/freespace.c --- postgresql-8.0.1/src/backend/storage/freespace/freespace.c2004-12-31 14:00:54.0 -0800 +++ postgresql-patched/src/backend/storage/freespace/freespace.c 2005-02-27 11:54:39.776546200 -0800 @@ -705,12 +705,25 @@ /* Convert stats to actual number of page slots needed */ needed = (sumRequests + numRels) * CHUNKPAGES; -ereport(elevel, -(errmsg(free space map: %d relations, %d pages stored; %.0f total pages needed, +ereport(INFO, +(errmsg(free space map: %d relations, %d pages stored; %.0f total pages used, numRels, storedPages, needed), - errdetail(Allocated FSM size: %d relations + %d pages = %.0f kB shared memory., + errdetail(FSM size: %d relations + %d pages = %.0f kB shared memory., MaxFSMRelations, MaxFSMPages, (double) FreeSpaceShmemSize() / 1024.0))); + +if (numRels == MaxFSMRelations) +ereport(LOG, +(errmsg(max_fsm_relations(%d) equals the number of relations checked, + MaxFSMRelations), + errhint(You have = %d relations. You should increase max_fsm_relations.,numRels))); +else +if (needed MaxFSMPages) +
Re: [PATCHES] default to WITHOUT OIDS
!gettext_noop(Create new tables with OIDs by default.), without? Neil Conway [EMAIL PROTECTED] This patch makes default_with_oids disabled by default, per earlier discussion, and updates the documentation accordingly. I might have missed a few spots in the documentation that implicitly assume that OIDs are present in user tables by default, but I think I got most of them. Barring any objections, I'll apply this on Monday. -Neil Index: doc/src/sgml/datatype.sgml === RCS file: /var/lib/cvs/pgsql/doc/src/sgml/datatype.sgml,v retrieving revision 1.155 diff -c -r1.155 datatype.sgml *** doc/src/sgml/datatype.sgml 22 Jan 2005 22:56:35 - 1.155 --- doc/src/sgml/datatype.sgml 11 Mar 2005 05:35:45 - *** *** 2980,2989 para Object identifiers (OIDs) are used internally by productnamePostgreSQL/productname as primary keys for various ! system tables. An OID system column is also added to user-created ! tables, unless literalWITHOUT OIDS/literal is specified when ! the table is created, or the xref linkend=guc-default-with-oids ! configuration variable is set to false. Type typeoid/ represents an object identifier. There are also several alias types for typeoid/: typeregproc/, typeregprocedure/, typeregoper/, typeregoperator/, typeregclass/, and --- 2980,2989 para Object identifiers (OIDs) are used internally by productnamePostgreSQL/productname as primary keys for various ! system tables. OIDs are not added to user-created tables, unless ! literalWITH OIDS/literal is specified when the table is ! created, or the xref linkend=guc-default-with-oids ! configuration variable is set to true. Type typeoid/ represents an object identifier. There are also several alias types for typeoid/: typeregproc/, typeregprocedure/, typeregoper/, typeregoperator/, typeregclass/, and *** *** 3000,3027 references to system tables. /para -note - para - OIDs are included by default in user-created tables in - productnamePostgreSQL/productname version;. However, this - behavior is likely to change in a future version of - productnamePostgreSQL/productname. Eventually, user-created - tables will not include an OID system column unless literalWITH - OIDS/literal is specified when the table is created, or the - varnamedefault_with_oids/varname configuration variable is set - to true. If your application requires the presence of an OID - system column in a table, it should specify literalWITH - OIDS/literal when that table is created to ensure compatibility - with future releases of productnamePostgreSQL/productname. - /para -/note - para The typeoid/ type itself has few operations beyond comparison. ! It can be cast to ! integer, however, and then manipulated using the standard integer ! operators. (Beware of possible signed-versus-unsigned confusion ! if you do this.) /para para --- 3000,3010 references to system tables. /para para The typeoid/ type itself has few operations beyond comparison. ! It can be cast to integer, however, and then manipulated using the ! standard integer operators. (Beware of possible ! signed-versus-unsigned confusion if you do this.) /para para Index: doc/src/sgml/ddl.sgml === RCS file: /var/lib/cvs/pgsql/doc/src/sgml/ddl.sgml,v retrieving revision 1.39 diff -c -r1.39 ddl.sgml *** doc/src/sgml/ddl.sgml 22 Jan 2005 22:56:35 - 1.39 --- doc/src/sgml/ddl.sgml 11 Mar 2005 05:41:25 - *** *** 868,878 primaryOID/primary secondarycolumn/secondary /indexterm ! The object identifier (object ID) of a row. This is a serial ! number that is automatically added by ! productnamePostgreSQL/productname to all table rows (unless ! the table was created using literalWITHOUT OIDS/literal, in which ! case this column is not present). This column is of type typeoid/type (same name as the column); see xref linkend=datatype-oid for more information about the type. /para --- 868,877 primaryOID/primary secondarycolumn/secondary /indexterm ! The object identifier (object ID) of a row. This column is only ! present if the table was created using literalWITH ! OIDS/literal, or if the xref linkend=guc-default-with-oids ! configuration variable was enabled. This column is of type typeoid/type (same name as the column); see xref
Re: [PATCHES] default to WITHOUT OIDS
Qingqing Zhou wrote: !gettext_noop(Create new tables with OIDs by default.), without? Right: if the variable is on, Postgres will create new tables with OIDs by default. If the variable is off, it won't. This is similar to how the check_function_bodies GUC var is already documented. (Speaking of which, it would be good if the descriptions of the GUC variables were more consistent with one another. Some say that a variable Sets the threshold of FROM items beyond which GEQO will be used, for example, whereas others merely state what the variable controls -- e.g. Encrypt passwords.) -Neil ---(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] pgcrypto: openssl digest fix
Marko Kreen wrote: Some builds (depends on crypto engine support?) of OpenSSL 0.9.7x have EVP_DigestFinal function which which clears all of EVP_MD_CTX. This makes pgcrypto crash in functions which re-use one digest context several times: hmac() and crypt() with md5 algorithm. Following patch fixes it by carring the digest info around EVP_DigestFinal and re-initializing cipher. Applied to HEAD, REL8_0_STABLE and REL7_4_STABLE. Thanks for the patch. Please apply this also to stable branches (8.0 / 7.4). Should it be backpatched to 7.3 and 7.2 as well? -Neil ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org