[HACKERS] Potential problem with HOT and indexes?
In answering the recent question on -general I reread README.HOT and had a couple thoughts: Practically, we prevent certain transactions from using the new index by setting pg_index.indcheckxmin to TRUE. Transactions are allowed to use such an index only after pg_index.xmin is below their TransactionXmin horizon, thereby ensuring that any incompatible rows in HOT chains are dead to them. (pg_index.xmin will be the XID of the CREATE INDEX transaction. The reason for using xmin rather than a normal column is that the regular vacuum freezing mechanism will take care of converting xmin to FrozenTransactionId before it can wrap around.) So it occurs to me that freezing xmin won't actually do what we want for indexcheckxmin. Namely it'll make the index *never* be used. I'm not sure what we would want to happen in this admittedly pretty unlikely scenario. We don't actually have anything protecting against having transactions with xmin much older than freeze_threshold still hanging around. This means in particular that the transaction creating the index will be unable to use the index if the transaction has old snapshots. We alleviate that problem somewhat by not setting indcheckxmin unless the table actually contains HOT chains with RECENTLY_DEAD members. In index.c: * A side effect is to set indexInfo-ii_BrokenHotChain to true if we detect * any potentially broken HOT chains. Currently, we set this if there are * any RECENTLY_DEAD entries in a HOT chain, without trying very hard to * detect whether they're really incompatible with the chain tip. I wonder if this particular case is good evidence that we need to be cleverer about checking if the indexed fields have actually changed. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] status of remaining patches
Robert Haas escribió: As far as I can tell the patch author has responded to all comments and pretty much done everything right. I haven't even looked at it enough to understand what it does or why I should care, but AFAICS it's had more interest and more reviewing than 90% of what was submitted for this CommitFest. I noticed but never put in email that it adds a #include postgres.h to some other header file, which should just be removed. I also noticed that the followup version posted by someone else than the OP did not include the readahead.c file (I tried it with the one last posted by Koichi Suzuki and it seemed to compile with no problems, although I didn't run it) I then started wondering about the API; why are all the callers checking the readahead module if there's space and then adding stuff in? ISTM they should just attempt to add the element, and readahead.c be in charge of determining whether there is space or not, internally. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Redefine _() to dgettext()instead of gettext() so that it uses
Hiroshi Saito wrote: Hi Alvaro-san. Yeah, It seems that it saves also except pl. then, It also regards me as very good. I tested just now, of course, it is very comfortable. :-) Thanks, Hiroshi-san. I just applied that last version. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential problem with HOT and indexes?
Gregory Stark st...@enterprisedb.com writes: So it occurs to me that freezing xmin won't actually do what we want for indexcheckxmin. Namely it'll make the index *never* be used. How do you figure that? FrozenXID is certainly in the past from any vantage point. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential problem with HOT and indexes?
Tom Lane t...@sss.pgh.pa.us writes: Gregory Stark st...@enterprisedb.com writes: So it occurs to me that freezing xmin won't actually do what we want for indexcheckxmin. Namely it'll make the index *never* be used. How do you figure that? FrozenXID is certainly in the past from any vantage point. Uhm, I'm not sure what I was thinking. Another thought now though. What if someone updates the pg_index entry -- since we never reset indcheckxmin then the new tuple will have a new xmin and will suddenly become invisible again for no reason. Couldn't this happen if you set a table WITHOUT CLUSTER for example? Or if --as possibly happened in the user's case-- you reindex the table and don't find any HOT update chains but the old pg_index entry had indcheckxmin set already? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential problem with HOT and indexes?
Gregory Stark st...@enterprisedb.com writes: Another thought now though. What if someone updates the pg_index entry -- since we never reset indcheckxmin then the new tuple will have a new xmin and will suddenly become invisible again for no reason. Hmm ... if updates to pg_index entries were common then I could get worried about that, but they really aren't. Couldn't this happen if you set a table WITHOUT CLUSTER for example? Or if --as possibly happened in the user's case-- you reindex the table and don't find any HOT update chains but the old pg_index entry had indcheckxmin set already? This is all useless guesswork until we find out whether he was using REINDEX or CREATE INDEX CONCURRENTLY. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Potential problem with HOT and indexes?
Tom Lane t...@sss.pgh.pa.us writes:
Gregory Stark st...@enterprisedb.com writes:
Another thought now though. What if someone updates the pg_index entry --
since we never reset indcheckxmin then the new tuple will have a new xmin and
will suddenly become invisible again for no reason.
Hmm ... if updates to pg_index entries were common then I could get
worried about that, but they really aren't.
Fixing this for REINDEX is fairly straightforward I think. It already updates
the pg_index line to fix indisvalid and indisready. see:
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 8b14b96..c12bf6c 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2332,6 +2332,9 @@ reindex_index(Oid indexId)
* If the index is marked invalid or not ready (ie, it's from a failed
* CREATE INDEX CONCURRENTLY), we can now mark it valid. This allows
* REINDEX to be used to clean up in such cases.
+*
+* Also if the index was originally built using CREATE INDEX
CONCURRENTLY
+* we want to clear the indcheckxmin field since it's no longer
relevant.
*/
pg_index = heap_open(IndexRelationId, RowExclusiveLock);
@@ -2342,10 +2345,11 @@ reindex_index(Oid indexId)
elog(ERROR, cache lookup failed for index %u, indexId);
indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
- if (!indexForm-indisvalid || !indexForm-indisready)
+ if (!indexForm-indisvalid || !indexForm-indisready ||
indexForm-indcheckxmin)
{
indexForm-indisvalid = true;
indexForm-indisready = true;
+ indexForm-indcheckxmin = false;
simple_heap_update(pg_index, indexTuple-t_self, indexTuple);
CatalogUpdateIndexes(pg_index, indexTuple);
}
Couldn't this happen if you set a table WITHOUT CLUSTER for example? Or if
--as possibly happened in the user's case-- you reindex the table and don't
find any HOT update chains but the old pg_index entry had indcheckxmin set
already?
This is all useless guesswork until we find out whether he was using
REINDEX or CREATE INDEX CONCURRENTLY.
Well he said he had a nightly REINDEX script. What's unknown is whether the
index was originally built with CREATE INDEX CONCURRENTLY. But I don't know
any other reason for a newly built index to go unused when the query is very
selective and then to suddenly start being used after a restart.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Out parameters handling
On Sat, Mar 7, 2009 at 9:29 PM, Dimitri Fontaine dfonta...@hi-media.comwrote: In fact, maybe a new option to set the OUT parameters prefix to use from within the function body would do? Le 7 mars 09 à 19:56, Dimitri Fontaine a écrit : CREATE OR REPLACE FUNCTION test_out ( IN a integer, IN b integer, OUT s integer ) RETURNS setof integer SET out_prefix TO 'v_' LANGUAGE PLPGSQL AS $f$ That's what we also would like to have. In addition it should also make out parameters unusable without that prefix. Then we could make it our coding standard and feel relatively safe again. Those two following lines would be deprecated: DECLARE v_s ALIAS FOR $3; BEGIN FOR v_s IN SELECT generate_series(a, b) LOOP v_s := v_s * v_s; RETURN NEXT; END LOOP; RETURN; END; $f$; CREATE FUNCTION dim=# SELECT * FROM test_out(2, 4); s 4 9 16 (3 rows) -- dim
Re: [HACKERS] ERROR: failed to locate grouping columns
Em Sáb, 2009-03-07 às 19:38 -0500, Tom Lane escreveu: I really have a hard time believing that whether you get that error is contingent on whether the view returns some rows or not. That's a planner message and couldn't possibly have to do with what happens at runtime. Well, today I have more time to study the environment and I'd see that was a coincidence in the fact that when the grouping by in the view works fine and it was returning values, it was tested in a 8.1.4 PG version. Now I made a complete test in 8.1.4 and 8.3.6. In the first the error not occurs, in the last yes. Would you put together a complete example, instead of leaving us to guess what's underlying the view? And what PG version is this? Attached there is a dump with the tables and views related: vw_cnt_vnc_tst - is my view before I changed sub-selects to JOIN vw_that_works - an example view that works without grouping some columns vw_that_not_works - an example view that throws an error thanks. -- Dickson S. Guedes - mail/xmpp: gue...@guedesoft.net - skype: guediz http://guedesoft.net - http://planeta.postgresql.org.br test_error_failed_to_locate_grouping_columns.dmp.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR: failed to locate grouping columns
Dickson S. Guedes lis...@guedesoft.net writes: Em Sáb, 2009-03-07 às 19:38 -0500, Tom Lane escreveu: Would you put together a complete example, instead of leaving us to guess what's underlying the view? And what PG version is this? Attached there is a dump with the tables and views related: vw_that_works - an example view that works without grouping some columns vw_that_not_works - an example view that throws an error OK, reproduced here on HEAD: dg=# select * from vw_that_not_works; ERROR: failed to locate grouping columns Off to do some debugging. Thanks for the test case! regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] status of remaining patches
Josh Berkus j...@agliodbs.com writes: I don't think this one is that far away either. I've been holding Bryce and Ramon's feet to the fire on the issue of possible downside, but so far there's not really much evidence of any *actual* as opposed to theoretical downside. What sorts of operations would we test which could potentially show a performance downside? I have to admit I don't really understand what use-cases this patch is meant to improve. The patch is meant to improve performance in cases where the outer relation's key distribution is heavily skewed, by introducing a fast path for keys matching the outer's most common values (MCVs). But it does that by potentially sacrificing performance for non MCV keys. So the case that's of concern is where the distribution is just skewed enough to trigger the patch's behavioral change, but you don't actually get a win because there are too many non-MCV keys. Note that as it's coded, the outer relation's skew is what triggers the behavioral change. It's not real clear to me how skew in the inner relation's distribution affects things. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Additional DTrace Probes
Zdenek Kotala wrote:
I tested your patch and it looks good. however I have several
comments/questions:
1) probes contains pointer but in probe.h it is declared as int. Is it
correct?
The probes cast the pointer to uintptr_t, so the correct type that will
work for both ILP32 and LP64 models is unsigned long. I've made the
correction from unsigned int to unsigned long. The reason uintptr_t is
not used in the probe definition is because it causes compilation
problem on Mac OS X. This is a known problem in the OS X's DTrace
implementation.
2) Maybe
TRACE_POSTGRESQL_SLRU_READPAGE_PHYSICAL_DONE(true, -1, -1);
would be better. Because slru_errcause, slru_errno can contains garbage
in situation when everything goes fine. Same for write.
I've made the changes per your suggestion although one can argue that
the script can check arg0, and if it's true, avoid using arg1 and arg2
as they are meaningless.
I think it is committable for 8.4.
That would be awesome!
-Robert
Index: src/backend/access/transam/slru.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/slru.c,v
retrieving revision 1.45
diff -u -3 -p -r1.45 slru.c
--- src/backend/access/transam/slru.c 1 Jan 2009 17:23:36 - 1.45
+++ src/backend/access/transam/slru.c 8 Mar 2009 20:39:01 -
@@ -57,6 +57,7 @@
#include storage/fd.h
#include storage/shmem.h
#include miscadmin.h
+#include pg_trace.h
/*
@@ -372,6 +373,7 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
{
SlruShared shared = ctl-shared;
+ TRACE_POSTGRESQL_SLRU_READPAGE_START((uintptr_t)ctl, pageno, write_ok,
xid);
/* Outer loop handles restart if we must wait for someone else's I/O */
for (;;)
{
@@ -399,6 +401,7 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
}
/* Otherwise, it's ready to use */
SlruRecentlyUsed(shared, slotno);
+ TRACE_POSTGRESQL_SLRU_READPAGE_DONE(slotno);
return slotno;
}
@@ -446,6 +449,7 @@ SimpleLruReadPage(SlruCtl ctl, int pagen
SlruReportIOError(ctl, pageno, xid);
SlruRecentlyUsed(shared, slotno);
+ TRACE_POSTGRESQL_SLRU_READPAGE_DONE(slotno);
return slotno;
}
}
@@ -470,6 +474,8 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl,
SlruShared shared = ctl-shared;
int slotno;
+ TRACE_POSTGRESQL_SLRU_READPAGE_READONLY((uintptr_t)ctl, pageno, xid);
+
/* Try to find the page while holding only shared lock */
LWLockAcquire(shared-ControlLock, LW_SHARED);
@@ -511,6 +517,8 @@ SimpleLruWritePage(SlruCtl ctl, int slot
int pageno = shared-page_number[slotno];
boolok;
+ TRACE_POSTGRESQL_SLRU_WRITEPAGE_START((uintptr_t)ctl, pageno, slotno);
+
/* If a write is in progress, wait for it to finish */
while (shared-page_status[slotno] == SLRU_PAGE_WRITE_IN_PROGRESS
shared-page_number[slotno] == pageno)
@@ -525,7 +533,10 @@ SimpleLruWritePage(SlruCtl ctl, int slot
if (!shared-page_dirty[slotno] ||
shared-page_status[slotno] != SLRU_PAGE_VALID ||
shared-page_number[slotno] != pageno)
+ {
+ TRACE_POSTGRESQL_SLRU_WRITEPAGE_DONE();
return;
+ }
/*
* Mark the slot write-busy, and clear the dirtybit. After this point,
a
@@ -569,6 +580,8 @@ SimpleLruWritePage(SlruCtl ctl, int slot
/* Now it's okay to ereport if we failed */
if (!ok)
SlruReportIOError(ctl, pageno, InvalidTransactionId);
+
+ TRACE_POSTGRESQL_SLRU_WRITEPAGE_DONE();
}
/*
@@ -593,6 +606,8 @@ SlruPhysicalReadPage(SlruCtl ctl, int pa
SlruFileName(ctl, path, segno);
+ TRACE_POSTGRESQL_SLRU_READPAGE_PHYSICAL_START((uintptr_t)ctl, path,
pageno, slotno);
+
/*
* In a crash-and-restart situation, it's possible for us to receive
* commands to set the commit status of transactions whose bits are in
@@ -607,6 +622,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pa
{
slru_errcause = SLRU_OPEN_FAILED;
slru_errno = errno;
+ TRACE_POSTGRESQL_SLRU_READPAGE_PHYSICAL_DONE(false,
slru_errcause, slru_errno);
return false;
}
@@ -614,6 +630,7 @@ SlruPhysicalReadPage(SlruCtl ctl, int pa
(errmsg(file \%s\ doesn't exist, reading as
zeroes,
path)));
MemSet(shared-page_buffer[slotno], 0, BLCKSZ);
+ TRACE_POSTGRESQL_SLRU_READPAGE_PHYSICAL_DONE(true, -1, -1);
return true;
}
@@ -622,6 +639,7 @@
Re: [HACKERS] pg_hba.conf - patch to report all parsing errors, and then bail
Magnus Hagander wrote: Applied. //Magnus Thanks. And thanks, Tom, for pointing out my multiple attempts to free. -selena -- Selena Deckelmann End Point Corporation sel...@endpoint.com 503-282-2512 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid MSVC breakage caused by my previous commit by not using a
Alvaro Herrera wrote: Tom Lane wrote: alvhe...@postgresql.org (Alvaro Herrera) writes: Avoid MSVC breakage caused by my previous commit by not using a variable in the src/bin/scripts Makefile. Buildfarm says it's still broken. Hmm, yeah, apparently Mkvcbuild.pm needs to be updated with the list of files that need to be symlinked -- and it's not all that straightforward. I need some help here, as I have no way to test this. This should be fixed per my commit a minute ago. Thanks for help over IM to figure it out :-) //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] status of remaining patches
Tom, I don't think this one is that far away either. I've been holding Bryce and Ramon's feet to the fire on the issue of possible downside, but so far there's not really much evidence of any *actual* as opposed to theoretical downside. What sorts of operations would we test which could potentially show a performance downside? I have to admit I don't really understand what use-cases this patch is meant to improve. --Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] status of remaining patches
Robert, The original patch was submitted by Koichi Suzuki - quite a few other people have looked at it and provided comments. Simon Riggs was assigned as the original reviewer, but for some reason Dave Page removed his name from the wiki a few days ago (I'm fixing this now). Actually, this patch has been reviewed by a whole slough of people. That's because Simon has said that he no longer has time to review it. --Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Out parameters handling
Hello Robert, I have been bitten by this problem many times as well. I wonder whether it would be possible to make PL/pgsql take :foo to mean the parameter named foo, and then provide an option to make that THE ONLY WAY to refer to the parameter foo. For backward-compatibility, and compatibility with (ahem) other database products, we probably don't want to remove the option to have foo mean... any damn thing named foo you can put your hands on. But it would be nice to at least have the option of disabling that behavior when compatibility is not an issue, and correctness is. This is one of the things I wanted to start looking at for 8.5. My idea was to optionally use : or @ (not sure which is more popular) to specify this token is only a variable. Do not try to match it to columns or other database object. If the variable did not start with : or @ then normal rules would apply for backwards compatibility. No idea how feasible this plan is, I was just hoping to find a way to solve this problem. Thanks, - Ryan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] compiler failures on buildfarm member wombat
Hi, Wombat has dyed of internal compiler errors twice lately: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=wombatdt=2009-03-08%2004:30:01 gistget.c: In function 'gistnext': gistget.c:358: internal compiler error: Segmentation fault Please submit a full bug report, with preprocessed source if appropriate. See http://bugs.gentoo.org/ for instructions. It has failed in different ways in the past, for example http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=wombatdt=2008-12-31%2004:30:01 ccache gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing -fwrapv -g -fpic pg_regress.o pg_regress_main.o -L../../../src/port -Wl,--as-needed -Wl,-rpath,'/home/markwkm/local/pgfarmbuild/HEAD/inst/lib' -lpgport -lxslt -lxml2 -lssl -lcrypto -lkrb5 -lz -lreadline -lcrypt -ldl -lm -o pg_regress collect2: ld terminated with signal 11 [Segmentation fault] make[2]: *** [pg_regress] Error 1 Could this be due to hardware problems? -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ERROR: failed to locate grouping columns
OK, I poked into this. The test case can be simplified to this: regression=# create table t1 (f1 numeric(14,0), f2 varchar(30)); CREATE TABLE regression=# create view vv as select distinct f1,f2,(select f2 from t1 x where x.f1=aa.f1) as fs from t1 aa; CREATE VIEW regression=# select * from vv group by f1,f2,fs; ERROR: failed to locate grouping columns The reason that locate_grouping_columns fails is that it's being asked to match up a Var with type varchar(30) (representing the result of the view's fs column) to a Var with typmod -1, and those are not equal according to equal(). The Var with default typmod is being manufactured by build_physical_tlist(), which is looking at a subquery RTE whose targetlist contains a SubPlan node. Since exprTypmod just punts on SubPlans, it constructs a Var with typmod -1. So there are a couple of places we could assign blame here: 1. Subqueries in RTE nodes are supposed to be virgin, unplanned querytrees, so finding a SubPlan in the targetlist is unexpected. On this theory, the fault is that of set_subquery_pathlist(), which ought to copy the RTE's subquery before it turns subquery_planner loose on it (not to mention the changes it itself makes...). More generally it's another reason to fix the planner to not scribble on its input, but that's a task for some other day. 2. It would still work if only SubPlans didn't lose information relative to SubLinks. On this theory we ought to add a firstColTypmod field to SubPlan. (The reason we didn't see this behavior before 8.3 is that exprTypmod punted on SubLinks, too, before 8.3, and so the output of the calling view would have been assigned typmod -1 anyway.) Solution #1 is a bit annoying from a planner performance point of view, but is probably the safest thing in the near term. Solution #2 is seeming like a good idea in the long run; but it also seems like it is just fixing one symptom of the general issue that we're scribbling on the content of a subquery RTE. I'm also a tad hesitant to back-patch it because I'm not sure if there are any places where it would change user-visible behavior in unexpected ways. So what I'm inclined to do is insert a copyObject() call into set_subquery_pathlist(), and maybe in the future add a typmod field to SubPlan. I remain a bit uncertain about how far back to back-patch. We know that 8.3 is broken and that 8.2 and before do not exhibit this particular symptom. It seems like there might be other problems with the same root cause that do afflict pre-8.3 versions, but if we've gone this long without finding them, are they really there? Should we slow down the planner in back versions to prevent a hypothetical problem? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Additional DTrace Probes
ITAGAKI Takahiro wrote: This will introduce SLRU and Executor probes. We already have Lock, LWLock, Buffer, I/O and XLogs probes. I'd like to have the following probes, too. In my experience, they could be bottlenecks or consume much time in some situations. - idle in transaction - spinlock wait-and-retry - CPU: Trigger execution - CPU: Encoding conversion - Network: send() and recv() - smgr: lseek() calls - Tempfile reads and writes Great suggestions. If you have particular use cases in mind, please send them to the list. This will help determine what kind of data to be made available via the probes. It'll also be helpful if you know the locations for the probes. -Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] postgresql.conf: patch to have ParseConfigFile report all parsing errors, then bail
ParseConfigFile currently exits on the first parsing error. Changed
guc_file.l to report all parsing errors before exiting:
* Moved parse_error: block inside while() loop
* Removed cleanup_exit: and associated 'goto'
* Added ereport if ParseConfigFile() returns false
* changed OK to ok ;)
* Added comment - TODO: Report bogus variables in addition to parsing
errors before bailing out
-selena
--
Selena Deckelmann
End Point Corporation
sel...@endpoint.com
503-282-2512
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
***
*** 143,149 ProcessConfigFile(GucContext context)
--- 143,155
if (!ParseConfigFile(ConfigFileName, NULL,
0, context, elevel,
head, tail))
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
+errmsg(Did not reload \%s\ due to earlier
parsing error(s), ConfigFileName)));
+ /* TODO: Report bogus variables in addition to parsing errors
before bailing out */
goto cleanup_list;
+ }
/*
* We need the proposed new value of custom_variable_classes to check
***
*** 361,367 ParseConfigFile(const char *config_file, const char
*calling_file,
struct name_value_pair **head_p,
struct name_value_pair **tail_p)
{
! boolOK = true;
charabs_path[MAXPGPATH];
FILE *fp;
YY_BUFFER_STATE lex_buffer;
--- 367,373
struct name_value_pair **head_p,
struct name_value_pair **tail_p)
{
! boolok = true;
charabs_path[MAXPGPATH];
FILE *fp;
YY_BUFFER_STATE lex_buffer;
***
*** 461,472 ParseConfigFile(const char *config_file, const char
*calling_file,
if (!ParseConfigFile(opt_value, config_file,
depth + 1,
context, elevel,
head_p,
tail_p))
! {
! pfree(opt_name);
! pfree(opt_value);
! OK = false;
! goto cleanup_exit;
! }
yy_switch_to_buffer(lex_buffer);
ConfigFileLineno = save_ConfigFileLineno;
pfree(opt_name);
--- 467,474
if (!ParseConfigFile(opt_value, config_file,
depth + 1,
context, elevel,
head_p,
tail_p))
! ok = false;
!
yy_switch_to_buffer(lex_buffer);
ConfigFileLineno = save_ConfigFileLineno;
pfree(opt_name);
***
*** 525,552 ParseConfigFile(const char *config_file, const char
*calling_file,
/* break out of loop if read EOF, else loop for next line */
if (token == 0)
break;
- }
! /* successful completion of parsing */
! goto cleanup_exit;
! parse_error:
! if (token == GUC_EOL || token == 0)
! ereport(elevel,
! (errcode(ERRCODE_SYNTAX_ERROR),
!errmsg(syntax error in file \%s\ line %u,
near end of line,
! config_file, ConfigFileLineno -
1)));
! else
! ereport(elevel,
! (errcode(ERRCODE_SYNTAX_ERROR),
!errmsg(syntax error in file \%s\ line %u,
near token \%s\,
! config_file, ConfigFileLineno,
yytext)));
! OK = false;
! cleanup_exit:
yy_delete_buffer(lex_buffer);
FreeFile(fp);
! return OK;
}
--- 527,555
/* break out of loop if read EOF, else loop for next line */
if (token == 0)
break;
! /* skip over parse_error if we made it this far without error */
! continue;
! parse_error:
! if (token == GUC_EOL || token == 0)
! ereport(elevel,
! (errcode(ERRCODE_SYNTAX_ERROR),
!errmsg(syntax error in file
\%s\ line %u, near end of line,
! config_file,
ConfigFileLineno - 1)));
!
Re: [HACKERS] compiler failures on buildfarm member wombat
Alvaro Herrera alvhe...@commandprompt.com writes: Wombat has dyed of internal compiler errors twice lately: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=wombatdt=2009-03-08%2004:30:01 gistget.c: In function 'gistnext': gistget.c:358: internal compiler error: Segmentation fault Please submit a full bug report, with preprocessed source if appropriate. See http://bugs.gentoo.org/ for instructions. Could this be due to hardware problems? Given wombat's very long history of usually-unrepeatable compiler crashes, I think it's probably hardware. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Out parameters handling
Ryan Bradetich rbradet...@gmail.com writes: This is one of the things I wanted to start looking at for 8.5. My idea was to optionally use : or @ (not sure which is more popular) to specify this token is only a variable. This whole line of thought is really a terrible idea IMHO. plpgsql is supposed to follow Oracle's pl/sql syntax, not invent random syntax of its own. I believe that 80% of the problems here are occurring because we used a crude substitution method that got the priorities backwards from the way Oracle does it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgresql.conf: patch to have ParseConfigFile report all parsing errors, then bail
Selena Deckelmann wrote: ! parse_error: ! if (token == GUC_EOL || token == 0) ! ereport(elevel, ! (errcode(ERRCODE_SYNTAX_ERROR), ! errmsg(syntax error in file \%s\ line %u, near end of line, ! config_file, ConfigFileLineno - 1))); Not that this has anything to do with the patch at hand, but I remember thinking about this sort of error message in the past. Would it be appropriate to move the file name and line number to an errcontext() field? I know somebody is going to say but errcontext is not shown when verbosity is set to terse, to which I say that we should fix that too. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Tom Lane wrote: I've seen a recent error that suggests we are clobbering memory somewhere in the parallel code, as well as Olivier Prennant's reported error that suggests the same thing, although I'm blessed if I can see where it might be. Maybe some more eyeballs on the code would help. Can you put together even a weakly reproducible test case? Something that only fails every tenth or hundredth time would still help. I have found the source of the problem I saw. dumputils.c:fmtId() uses a static PQExpBuffer which it initialises the first time it's called. This gets clobbered by simultaneous calls by Windows threads. I could just make it auto and set it up on each call, but that could result in a non-trivial memory leak ... it's probably called a great many times. Or I could provide a parallel version where we pass in a PQExpBuffer that we create, one per thread, and is used by anything called by the parallel code. That seems like a bit of a potential footgun, though. Has anyone got a better plan? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgresql.conf: patch to have ParseConfigFile report all parsing errors, then bail
Alvaro Herrera alvhe...@commandprompt.com writes: Not that this has anything to do with the patch at hand, but I remember thinking about this sort of error message in the past. Would it be appropriate to move the file name and line number to an errcontext() field? I think the message is fine as is. If you moved that information then you'd have nothing left in the base message but syntax error, which is useless. I know somebody is going to say but errcontext is not shown when verbosity is set to terse, to which I say that we should fix that too. No, because the whole point of terse is to be terse. Adding context to the output would just create a demand for super terse mode. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Andrew Dunstan and...@dunslane.net writes: I have found the source of the problem I saw. dumputils.c:fmtId() uses a static PQExpBuffer which it initialises the first time it's called. This gets clobbered by simultaneous calls by Windows threads. Ugh. But that doesn't explain the original trouble report on Unixware. I could just make it auto and set it up on each call, but that could result in a non-trivial memory leak ... it's probably called a great many times. Or I could provide a parallel version where we pass in a PQExpBuffer that we create, one per thread, and is used by anything called by the parallel code. That seems like a bit of a potential footgun, though. I think we should try hard to keep this localized to fmtId(), rather than changing the callers --- the latter would be a huge readability hit. Is there a reasonable way to have fmtId use thread-local storage for its PQExpBuffer pointer on Windows? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Andrew Dunstan wrote: I have found the source of the problem I saw. dumputils.c:fmtId() uses a static PQExpBuffer which it initialises the first time it's called. This gets clobbered by simultaneous calls by Windows threads. I could just make it auto and set it up on each call, but that could result in a non-trivial memory leak ... it's probably called a great many times. Or I could provide a parallel version where we pass in a PQExpBuffer that we create, one per thread, and is used by anything called by the parallel code. That seems like a bit of a potential footgun, though. Could you use a different static PQExpBuffer on each thread? pthread_getspecific sort of thing, only to be used on Windows. BTW does fmtQualifiedId have the same problem? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Alvaro Herrera wrote: Andrew Dunstan wrote: I have found the source of the problem I saw. dumputils.c:fmtId() uses a static PQExpBuffer which it initialises the first time it's called. This gets clobbered by simultaneous calls by Windows threads. I could just make it auto and set it up on each call, but that could result in a non-trivial memory leak ... it's probably called a great many times. Or I could provide a parallel version where we pass in a PQExpBuffer that we create, one per thread, and is used by anything called by the parallel code. That seems like a bit of a potential footgun, though. Could you use a different static PQExpBuffer on each thread? pthread_getspecific sort of thing, only to be used on Windows. For MSVC we could declare it with _declspec(thread) and it would be allocated in thread-local storage, but it looks like this isn't supported on Mingw. BTW does fmtQualifiedId have the same problem? Yes, but it's not called in threaded code - it's only in pg_dump and only pg_restore is threaded. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Andrew Dunstan and...@dunslane.net writes: Alvaro Herrera wrote: Could you use a different static PQExpBuffer on each thread? pthread_getspecific sort of thing, only to be used on Windows. For MSVC we could declare it with _declspec(thread) and it would be allocated in thread-local storage, but it looks like this isn't supported on Mingw. Some googling suggests that thread-local storage is supported on latest release (not clear if it's exactly the same syntax though :-(). Worst case, we could say that parallel restore isn't supported on mingw. I'm not entirely sure why we bother with that platform at all... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] small parallel restore optimization
Tom Lane wrote: Worst case, we could say that parallel restore isn't supported on mingw. I'm not entirely sure why we bother with that platform at all... I think you're confusing it with cygwin ... -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Sampling Profler for Postgres
Hello, I think we need two types of profilers: SQL-based and resource-based. We have some SQL-based profilers like slow-query logs (log_min_duration_statement) and contrib/pg_stat_statements in 8.4. For resource-based profilers, we have DTrace probes[1] and continue to extend them[2], but unfortunately DTrace only works on Solaris and limited platforms. Also, it is not so easy for typical users to write profilers using DTrace without performance degradation. [1] http://developer.postgresql.org/pgdocs/postgres/dynamic-trace.html [2] http://archives.postgresql.org/pgsql-hackers/2009-03/msg00226.php Therefore, I'd like to propose an profiler with sampling approach in 8.5. The attached patch is an experimental model of the profiler. Each backends reports its condtion in PgBackendStatus.st_condition and the stats collector process does polling them every seconds. This is an extension of the st_waiting field, which reports locking condition in pg_stat_activity. There are some advantages in portability and less overhead. Consideration is needed about how to coexist with DTrace. I added codes to push/pop conditions just on the same place as TRACE_POSTGRESQL_*_START/DONE(). So, we could merge the codes of DTrace and the profiler, or implement one of them with another. I would emphasize that an offical profler is required in this area because it enables users to share knowledge and documentaions; information-sharing would be difficult if they use home-made profilers. Comments welcome. Here is a sample output of the profiler with pgbench on Windows: $ pgbench -i -s3 $ psql -c SELECT pg_save_profiles() $ pgbench -c4 -T60 -n transaction type: TPC-B (sort of) tps = 401.510694 $ psql -c SELECT * FROM pg_diff_profiles profid | profname | percent ++- 19 | XLog:Write | 23.04- means wal contension 46 | LWLock:WALWrite| 23.04- same as the above 32 | Lock:Transaction | 22.61- confliction on row locks 15 | Network:Recv |7.83 21 | Data:Stat |4.35- lseek() is slow on Windows 7 | CPU:Execute|3.91 3 | CPU|3.91 1 | Idle:InTransaction |2.61 5 | CPU:Rewrite|1.74 16 | Network:Send |1.74 6 | CPU:Plan |1.74 31 | Lock:Tuple |1.74 4 | CPU:Parse |0.87 11 | CPU:Commit |0.87 (14 rows) Regards, --- ITAGAKI Takahiro NTT Open Source Software Center profiler_0309.tar.gz Description: Binary data additional_script.sql Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
