Re: [PATCHES] Index split WAL reduction
Here's an updated patch that fixes a bug with full_page_writes, and an
alignment issue in replay code. Also added a new test case to
crashtest.sh that exercises the fixed bug.
Has anyone looked at this? I now consider it ready for committing, if
no-one sees a problem with it.
Here's the original description of the patch:
Currently, an index split writes all the data on the split page to
WAL. That's a lot of WAL traffic. The tuples that are copied to the
right page need to be WAL logged, but the tuples that stay on the
original page don't.
Here's a patch to do that. It needs further testing, I have used the
attached crude crashtest.sh to test the basics, but we need to test
the more obscure cases like splitting non-leaf or root page.
On a test case that inserts 1 rows in increasing key order with a
100 characters wide text-field as key, the patch reduced the total
generated WAL traffic from 45MB to 33MB, or ~ 25%. Your mileage may
vary, depending on the tuple and key sizes, and the order of inserts.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
crashtest.sh
Description: application/shellscript
Index: src/backend/access/nbtree/nbtinsert.c
===
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/access/nbtree/nbtinsert.c,v
retrieving revision 1.146
diff -c -r1.146 nbtinsert.c
*** src/backend/access/nbtree/nbtinsert.c 11 Nov 2006 01:14:18 - 1.146
--- src/backend/access/nbtree/nbtinsert.c 2 Jan 2007 15:58:59 -
***
*** 733,738
--- 733,739
rightoff;
OffsetNumber maxoff;
OffsetNumber i;
+ bool isroot;
rbuf = _bt_getbuf(rel, P_NEW, BT_WRITE);
origpage = BufferGetPage(buf);
***
*** 747,752
--- 748,755
lopaque = (BTPageOpaque) PageGetSpecialPointer(leftpage);
ropaque = (BTPageOpaque) PageGetSpecialPointer(rightpage);
+ isroot = P_ISROOT(oopaque);
+
/* if we're splitting this page, it won't be the root when we're done */
/* also, clear the SPLIT_END and HAS_GARBAGE flags in both pages */
lopaque-btpo_flags = oopaque-btpo_flags;
***
*** 926,986
MarkBufferDirty(sbuf);
}
/* XLOG stuff */
if (!rel-rd_istemp)
{
xl_btree_split xlrec;
uint8 xlinfo;
XLogRecPtr recptr;
! XLogRecData rdata[4];
! xlrec.target.node = rel-rd_node;
! ItemPointerSet((xlrec.target.tid), itup_blkno, itup_off);
if (newitemonleft)
! xlrec.otherblk = BufferGetBlockNumber(rbuf);
else
! xlrec.otherblk = BufferGetBlockNumber(buf);
! xlrec.leftblk = lopaque-btpo_prev;
! xlrec.rightblk = ropaque-btpo_next;
! xlrec.level = lopaque-btpo.level;
! /*
* Direct access to page is not good but faster - we should implement
* some new func in page API. Note we only store the tuples
* themselves, knowing that the item pointers are in the same order
* and can be reconstructed by scanning the tuples. See comments for
* _bt_restore_page().
*/
! xlrec.leftlen = ((PageHeader) leftpage)-pd_special -
! ((PageHeader) leftpage)-pd_upper;
! rdata[0].data = (char *) xlrec;
! rdata[0].len = SizeOfBtreeSplit;
! rdata[0].buffer = InvalidBuffer;
! rdata[0].next = (rdata[1]);
!
! rdata[1].data = (char *) leftpage + ((PageHeader) leftpage)-pd_upper;
! rdata[1].len = xlrec.leftlen;
! rdata[1].buffer = InvalidBuffer;
! rdata[1].next = (rdata[2]);
!
! rdata[2].data = (char *) rightpage + ((PageHeader) rightpage)-pd_upper;
! rdata[2].len = ((PageHeader) rightpage)-pd_special -
((PageHeader) rightpage)-pd_upper;
! rdata[2].buffer = InvalidBuffer;
! rdata[2].next = NULL;
if (!P_RIGHTMOST(ropaque))
{
! rdata[2].next = (rdata[3]);
! rdata[3].data = NULL;
! rdata[3].len = 0;
! rdata[3].buffer = sbuf;
! rdata[3].buffer_std = true;
! rdata[3].next = NULL;
}
! if (P_ISROOT(oopaque))
xlinfo = newitemonleft ? XLOG_BTREE_SPLIT_L_ROOT : XLOG_BTREE_SPLIT_R_ROOT;
else
xlinfo = newitemonleft ? XLOG_BTREE_SPLIT_L : XLOG_BTREE_SPLIT_R;
--- 929,1044
MarkBufferDirty(sbuf);
}
+ /*
+ * By here, the original data page has been split into two new halves, and
+ * these are correct. The algorithm requires that the left page never
+ * move during a split, so we copy the new left page back on top of the
+ * original. Note that this is not a waste of time, since we also require
+ * (in the page management code) that the center of a page always be
+ * clean, and the most efficient way to guarantee this is just to compact
+ * the data by reinserting it into a new left page. (XXX the latter
+ * comment is probably obsolete.)
+ *
+ * We need to do this before writing the WAL record, so that XLogInsert can
+ * WAL log an image of the page if necessary.
+ */
+ PageRestoreTempPage(leftpage, origpage);
+
/* XLOG stuff */
if (!rel-rd_istemp)
{
Re: [HACKERS] [PATCHES] [BUGS] BUG #2846: inconsistent and
Applied. --- Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: OK, are you saying that there is a signal we are ignoring for overflow/underflow, or that we should just silently overflow/underflow and not throw an error? Silent underflow is fine with me; it's the norm in most all float implementations and won't surprise anyone. For overflow I'm OK with either returning infinity or throwing an error --- but if an error, it should only be about inf-out-with-non-inf-in, not comparisons to any artificial MAX/MIN values. OK, I am happy to remove the MIN/MAX comparisons. Those were in the original code. The attached, updated patch creates a single CHECKFLOATVAL() macro that does the overflow/underflow comparisons and throws an error. This also reduces the isinf() calls. Should I be concerned we are now duplicating the error text in all call sites? Regression wording modified now that float4/float8 checks are merged. I haven't update the platform-specific float* expected files yet, but will on commit. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] xlog directory at initdb time
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. Euler Taveira de Oliveira wrote: Hi, This simple patch lets someone specifies the xlog directory at initdb time. It uses symlinks to do it, and create and/or set permissions at the directory as appropriate. -- Euler Taveira de Oliveira http://www.timbira.com/ [ Attachment, skipping... ] ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Recent SIGSEGV failures in buildfarm HEAD
Tom Lane wrote:
Andrew Dunstan [EMAIL PROTECTED] writes:
... And then we'd need to change the regression makefile to use
the option, based on an environment variable a bit like MAX_CONNEXCTIONS
maybe.
Why wouldn't we just use it always? If a regression test dumps core,
that's going to deserve investigation.
Revised patch attached, doing just this. I will apply it soon unless
there are objections.
cheers
andrew
Index: doc/src/sgml/ref/pg_ctl-ref.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/pg_ctl-ref.sgml,v
retrieving revision 1.35
diff -c -r1.35 pg_ctl-ref.sgml
*** doc/src/sgml/ref/pg_ctl-ref.sgml2 Dec 2006 00:34:52 - 1.35
--- doc/src/sgml/ref/pg_ctl-ref.sgml2 Jan 2007 20:25:01 -
***
*** 29,34
--- 29,35
arg-l replaceablefilename/replaceable/arg
arg-o replaceableoptions/replaceable/arg
arg-p replaceablepath/replaceable/arg
+arg-c/arg
sbr
commandpg_ctl/command
arg choice=plainstop/arg
***
*** 48,53
--- 49,55
arg-w/arg
arg-s/arg
arg-D replaceabledatadir/replaceable/arg
+arg-c/arg
arg-m
group choice=plain
args[mart]/arg
***
*** 246,251
--- 248,266
/varlistentry
varlistentry
+ termoption-c/option/term
+ listitem
+para
+ Attempt to allow server crashes to produce core files, on platforms
+ where this available, by lifting any soft resource limit placed on
+ them.
+ This is useful in debugging or diagnosing problems by allowing
a
+ stack trace to be obtained from a failed server process.
+/para
+ /listitem
+ /varlistentry
+
+ varlistentry
termoption-w/option/term
listitem
para
Index: src/bin/pg_ctl/pg_ctl.c
===
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.74
diff -c -r1.74 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c 12 Oct 2006 05:14:49 - 1.74
--- src/bin/pg_ctl/pg_ctl.c 2 Jan 2007 20:25:02 -
***
*** 26,31
--- 26,36
#include sys/stat.h
#include unistd.h
+ #ifdef HAVE_SYS_RESOURCE_H
+ #include sys/time.h
+ #include sys/resource.h
+ #endif
+
#include libpq/pqsignal.h
#include getopt_long.h
***
*** 90,95
--- 95,103
static char *register_username = NULL;
static char *register_password = NULL;
static char *argv0 = NULL;
+ #if HAVE_GETRLIMIT
+ static bool allow_core_files = false;
+ #endif
static void
write_stderr(const char *fmt,...)
***
*** 132,137
--- 140,149
static char pid_file[MAXPGPATH];
static char conf_file[MAXPGPATH];
+ #if HAVE_GETRLIMIT
+ static void unlimit_core_size(void);
+ #endif
+
#if defined(WIN32) || defined(__CYGWIN__)
static void
***
*** 478,483
--- 490,516
}
+ #if HAVE_GETRLIMIT
+ static void
+ unlimit_core_size(void)
+ {
+ struct rlimit lim;
+ getrlimit(RLIMIT_CORE,lim);
+ if (lim.rlim_max == 0)
+ {
+ write_stderr(_(%s: cannot set core size,: disallowed
by hard limit.\n),
+progname);
+ return;
+ }
+ else if (lim.rlim_max == RLIM_INFINITY || lim.rlim_cur lim.rlim_max)
+ {
+ lim.rlim_cur = lim.rlim_max;
+ setrlimit(RLIMIT_CORE,lim);
+ }
+ }
+ #endif
+
+
static void
do_start(void)
***
*** 581,586
--- 614,624
postgres_path = postmaster_path;
}
+ #if HAVE_GETRLIMIT
+ if (allow_core_files)
+ unlimit_core_size();
+ #endif
+
exitcode = start_postmaster();
if (exitcode != 0)
{
***
*** 1401,1406
--- 1439,1447
printf(_( -o OPTIONS command line options to pass to
postgres\n
(PostgreSQL server
executable)\n));
printf(_( -p PATH-TO-POSTGRESnormally not necessary\n));
+ #if HAVE_GETRLIMIT
+ printf(_( -c, --corefilesallow postgres to produce core
files\n));
+ #endif
printf(_(\nOptions for stop or restart:\n));
printf(_( -m SHUTDOWN-MODE may be \smart\, \fast\, or
\immediate\\n));
***
*** 1497,1502
--- 1538,1546
{mode, required_argument, NULL, 'm'},
{pgdata, required_argument, NULL, 'D'},
{silent, no_argument, NULL, 's'},
+ #if HAVE_GETRLIMIT
+ {corefiles, no_argument, NULL, 'c'},
+ #endif
{NULL, 0, NULL, 0}
};
***
*** 1561,1567
/* process command-line options */
while (optind argc)
{
!
Re: [PATCHES] Patch(es) to expose n_live_tuples and
Patch applied. Thanks.
---
Glen Parker wrote:
This patch consists of two c functions to expose n_live_tuples and
n_dead_tuples, SQL functions to expose them to SQL land, and
corresponding fields added to pg_stat_all_tables.
This has been discussed in general. The purpose is to allow
autovacuum-esq conditional vacuuming and clustering using SQL to
discover the required stats.
-Glen Parker
--- ./src/backend/utils/adt/pgstatfuncs.c.old 2006-12-20 17:01:30.585852856
-0800
+++ ./src/backend/utils/adt/pgstatfuncs.c 2006-12-20 17:00:58.570719896
-0800
@@ -28,6 +28,8 @@
extern Datum pg_stat_get_tuples_inserted(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_updated(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_tuples_deleted(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_live_tuples(PG_FUNCTION_ARGS);
+extern Datum pg_stat_get_dead_tuples(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_blocks_hit(PG_FUNCTION_ARGS);
extern Datum pg_stat_get_last_vacuum_time(PG_FUNCTION_ARGS);
@@ -153,6 +155,38 @@
Datum
+pg_stat_get_live_tuples(PG_FUNCTION_ARGS)
+{
+ Oid relid = PG_GETARG_OID(0);
+ int64 result;
+ PgStat_StatTabEntry *tabentry;
+
+ if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ result = 0;
+ else
+ result = (int64) (tabentry-n_live_tuples);
+
+ PG_RETURN_INT64(result);
+}
+
+
+Datum
+pg_stat_get_dead_tuples(PG_FUNCTION_ARGS)
+{
+ Oid relid = PG_GETARG_OID(0);
+ int64 result;
+ PgStat_StatTabEntry *tabentry;
+
+ if ((tabentry = pgstat_fetch_stat_tabentry(relid)) == NULL)
+ result = 0;
+ else
+ result = (int64) (tabentry-n_dead_tuples);
+
+ PG_RETURN_INT64(result);
+}
+
+
+Datum
pg_stat_get_blocks_fetched(PG_FUNCTION_ARGS)
{
Oid relid = PG_GETARG_OID(0);
--- ./src/include/catalog/pg_proc.h.old 2006-12-06 10:06:47.0
-0800
+++ ./src/include/catalog/pg_proc.h 2006-12-20 17:09:32.874533832 -0800
@@ -2872,6 +2872,10 @@
DESCR(Statistics: Number of tuples updated);
DATA(insert OID = 1933 ( pg_stat_get_tuples_deleted PGNSP PGUID 12 f f t f
s 1 20 26 _null_ _null_ _null_ pg_stat_get_tuples_deleted - _null_ ));
DESCR(Statistics: Number of tuples deleted);
+DATA(insert OID = 2878 ( pg_stat_get_live_tuplesPGNSP PGUID 12 f f t f
s 1 20 26 _null_ _null_ _null_ pg_stat_get_live_tuples - _null_ ));
+DESCR(Statistics: Number of live tuples);
+DATA(insert OID = 2879 ( pg_stat_get_dead_tuplesPGNSP PGUID 12 f f t f
s 1 20 26 _null_ _null_ _null_ pg_stat_get_dead_tuples - _null_ ));
+DESCR(Statistics: Number of dead tuples);
DATA(insert OID = 1934 ( pg_stat_get_blocks_fetched PGNSP PGUID 12 f f t f
s 1 20 26 _null_ _null_ _null_ pg_stat_get_blocks_fetched - _null_ ));
DESCR(Statistics: Number of blocks fetched);
DATA(insert OID = 1935 ( pg_stat_get_blocks_hit PGNSP PGUID 12
f f t f s 1 20 26 _null_ _null_ _null_ pg_stat_get_blocks_hit - _null_ ));
--- ./src/backend/catalog/system_views.sql.old2006-12-06
10:06:47.0 -0800
+++ ./src/backend/catalog/system_views.sql2006-12-20 17:13:03.036584344
-0800
@@ -203,10 +203,12 @@
pg_stat_get_tuples_returned(C.oid) AS seq_tup_read,
sum(pg_stat_get_numscans(I.indexrelid))::bigint AS idx_scan,
sum(pg_stat_get_tuples_fetched(I.indexrelid))::bigint +
-pg_stat_get_tuples_fetched(C.oid) AS idx_tup_fetch,
+pg_stat_get_tuples_fetched(C.oid) AS idx_tup_fetch,
pg_stat_get_tuples_inserted(C.oid) AS n_tup_ins,
pg_stat_get_tuples_updated(C.oid) AS n_tup_upd,
pg_stat_get_tuples_deleted(C.oid) AS n_tup_del,
+pg_stat_get_live_tuples(C.oid) AS n_live_tup,
+pg_stat_get_dead_tuples(C.oid) AS n_dead_tup,
pg_stat_get_last_vacuum_time(C.oid) as last_vacuum,
pg_stat_get_last_autovacuum_time(C.oid) as last_autovacuum,
pg_stat_get_last_analyze_time(C.oid) as last_analyze,
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Recent SIGSEGV failures in buildfarm HEAD
Andrew Dunstan [EMAIL PROTECTED] writes: Revised patch attached, doing just this. I will apply it soon unless there are objections. Probably a good idea to check defined(HAVE_GETRLIMIT) defined(RLIMIT_CORE), rather than naively assuming every getrlimit implementation supports that particular setting. Also, should the -c option exist but just not do anything if the platform doesn't support it? As is, you're making it impossible to just specify -c without worrying if it does anything. The documentation fails to list the long form of the switch (--corefiles, which should probably really be --core-files for consistency). There's a typo in this message, too: + _(%s: cannot set core size,: disallowed by hard limit.\n), regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Recent SIGSEGV failures in buildfarm HEAD
Tom Lane wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Revised patch attached, doing just this. I will apply it soon unless there are objections. Probably a good idea to check defined(HAVE_GETRLIMIT) defined(RLIMIT_CORE), rather than naively assuming every getrlimit implementation supports that particular setting. Also, should the -c option exist but just not do anything if the platform doesn't support it? As is, you're making it impossible to just specify -c without worrying if it does anything. The documentation fails to list the long form of the switch (--corefiles, which should probably really be --core-files for consistency). There's a typo in this message, too: + _(%s: cannot set core size,: disallowed by hard limit.\n), OK, I'll fix all this. Thanks. cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
[PATCHES] Patch to log usage of temporary files
Thanks to Simon Riggs and Bruce for input that helped me put this together.
--
Bill Moran
Collaborative Fusion Inc.
diff -c -r src.orig/backend/storage/file/fd.c src/backend/storage/file/fd.c
*** src.orig/backend/storage/file/fd.c Thu Dec 7 15:44:42 2006
--- src/backend/storage/file/fd.c Tue Jan 2 12:33:25 2007
***
*** 50,55
--- 50,56
#include access/xact.h
#include storage/fd.h
#include storage/ipc.h
+ #include utils/guc.h
/*
***
*** 938,944
void
FileClose(File file)
{
! Vfd *vfdP;
Assert(FileIsValid(file));
--- 939,946
void
FileClose(File file)
{
! Vfd *vfdP;
! struct stat filestats;
Assert(FileIsValid(file));
***
*** 968,973
--- 970,984
{
/* reset flag so that die() interrupt won't cause problems */
vfdP-fdstate = ~FD_TEMPORARY;
+ if (stat(vfdP-fileName, filestats) == 0) {
+ if (trace_temp_files)
+ ereport(LOG,
+ (errmsg(temp file: size %lu path \%s\,
+ filestats.st_size, vfdP-fileName)));
+ PG_TRACE1(temp__file__cleanup, filestats.st_size);
+ } else {
+ elog(ERROR, Could not stat \%s\: %m, vfdP-fileName);
+ }
if (unlink(vfdP-fileName))
elog(LOG, failed to unlink \%s\: %m,
vfdP-fileName);
diff -c -r src.orig/backend/utils/misc/guc.c src/backend/utils/misc/guc.c
*** src.orig/backend/utils/misc/guc.c Wed Nov 29 09:50:07 2006
--- src/backend/utils/misc/guc.c Fri Dec 29 10:28:08 2006
***
*** 180,186
int log_min_messages = NOTICE;
int client_min_messages = NOTICE;
int log_min_duration_statement = -1;
!
int num_temp_buffers = 1000;
char *ConfigFileName;
--- 180,187
int log_min_messages = NOTICE;
int client_min_messages = NOTICE;
int log_min_duration_statement = -1;
! bool trace_temp_files = false;
!
int num_temp_buffers = 1000;
char *ConfigFileName;
***
*** 1010,1016
IgnoreSystemIndexes,
false, NULL, NULL
},
!
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
--- 1011,1027
IgnoreSystemIndexes,
false, NULL, NULL
},
!
! {
! {trace_temp_files, PGC_USERSET, LOGGING_WHAT,
! gettext_noop(Enables logging the usage of temp files.),
! gettext_noop(Size and location of each temp file is reported.),
! NULL
! },
! trace_temp_files,
! false, NULL, NULL
! },
!
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL
***
*** 1471,1477
log_min_duration_statement,
-1, -1, INT_MAX / 1000, NULL, NULL
},
!
{
{bgwriter_delay, PGC_SIGHUP, RESOURCES,
gettext_noop(Background writer sleep time between rounds in milliseconds),
--- 1482,1488
log_min_duration_statement,
-1, -1, INT_MAX / 1000, NULL, NULL
},
!
{
{bgwriter_delay, PGC_SIGHUP, RESOURCES,
gettext_noop(Background writer sleep time between rounds in milliseconds),
diff -c -r src.orig/backend/utils/misc/postgresql.conf.sample src/backend/utils/misc/postgresql.conf.sample
*** src.orig/backend/utils/misc/postgresql.conf.sample Mon Nov 20 20:23:37 2006
--- src/backend/utils/misc/postgresql.conf.sample Fri Dec 29 12:45:16 2006
***
*** 333,338
--- 333,339
#log_statement = 'none' # none, ddl, mod, all
#log_hostname = off
+ #trace_temp_files = off # Log usage of temporary files
#---
# RUNTIME STATISTICS
diff -c -r src.orig/include/utils/guc.h src/include/utils/guc.h
*** src.orig/include/utils/guc.h Thu Oct 19 14:32:47 2006
--- src/include/utils/guc.h Fri Dec 29 10:00:40 2006
***
*** 123,128
--- 123,129
extern int log_min_messages;
extern int client_min_messages;
extern int log_min_duration_statement;
+ extern bool trace_temp_files;
extern int num_temp_buffers;
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Patch to log usage of temporary files
Bill Moran wrote: Thanks to Simon Riggs and Bruce for input that helped me put this together. Please change things to save the stat() syscall when the feature is not in use. Nitpick: also note our brace placement convention (though this would be fixed by pgindent, but still). -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Patch to log usage of temporary files
Bill Moran wrote:
+ if (stat(vfdP-fileName, filestats) == 0) {
+ if (trace_temp_files)
Shouldn't these tests be the other way around?
cheers
andrew
---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at
http://www.postgresql.org/about/donate
Re: [PATCHES] Patch to log usage of temporary files
In response to Alvaro Herrera [EMAIL PROTECTED]: Bill Moran wrote: Thanks to Simon Riggs and Bruce for input that helped me put this together. Please change things to save the stat() syscall when the feature is not in use. Do you have a suggestion on how to do that and still have the PG_TRACE1() work? That was specifically requested by Simon Riggs. I'm not at all familiar with how the PG_TRACE probes work, so I'd be interested to hear suggestions on how to wrap that in an if. If I remove the PG_TRACE, it becomes a simple matter to skip the stat() if the feature is disabled. Nitpick: also note our brace placement convention (though this would be fixed by pgindent, but still). Sorry, I thought I _was_ following the convention. Must have missed something. Is there a written style guide somewhere? Might drive things home a little better than just looking at other folks code. -- Bill Moran Collaborative Fusion Inc. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Patch to log usage of temporary files
Bill Moran [EMAIL PROTECTED] writes:
In response to Alvaro Herrera [EMAIL PROTECTED]:
Please change things to save the stat() syscall when the feature is not
in use.
Do you have a suggestion on how to do that and still have the PG_TRACE1()
work? That was specifically requested by Simon Riggs.
Well, we are NOT paying a stat() call on every single file close,
whether Simon wants it or not. PG_TRACE1 doesn't even do anything
on non-Solaris platforms, for pete's sake.
Perhaps it would be reasonable to define trace_temp_files as the minimum
file size to log; then you could do something like
if (trace_temp_files 0)
{
if (stat(vfdP-fileName, filestats) 0)
elog(LOG, ...);
else
{
if (filestats.st_size / BLCKSZ = trace_temp_files)
ereport(LOG, ...);
PG_TRACE1(temp__file__cleanup, filestats.st_size);
}
}
Note that elog(ERROR) is quite inappropriate here.
regards, tom lane
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
