Re: [HACKERS] SSI bug?
I think I see what is going on now. We are sometimes failing to set the
commitSeqNo correctly on the lock. In particular, if a lock assigned to
OldCommittedSxact is marked with InvalidSerCommitNo, it will never be
cleared.
The attached patch corrects this:
TransferPredicateLocksToNewTarget should initialize a new lock
entry's commitSeqNo to that of the old one being transferred, or take
the minimum commitSeqNo if it is merging two lock entries.
Also, CreatePredicateLock should initialize commitSeqNo for to
InvalidSerCommitSeqNo instead of to 0. (I don't think using 0 would
actually affect anything, but we should be consistent.)
I also added a couple of assertions I used to track this down: a
lock's commitSeqNo should never be zero, and it should be
InvalidSerCommitSeqNo if and only if the lock is not held by
OldCommittedSxact.
Takashi, does this patch fix your problem with leaked SIReadLocks?
Dan
--
Dan R. K. Ports MIT CSAILhttp://drkp.net/
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index f6e49fe..d166da3 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -2067,7 +2067,7 @@ CreatePredicateLock(const PREDICATELOCKTARGETTAG *targettag,
SHMQueueInsertBefore(&(target->predicateLocks), &(lock->targetLink));
SHMQueueInsertBefore(&(sxact->predicateLocks),
&(lock->xactLink));
- lock->commitSeqNo = 0;
+ lock->commitSeqNo = InvalidSerCommitSeqNo;
}
LWLockRelease(partitionLock);
@@ -2500,6 +2500,7 @@ TransferPredicateLocksToNewTarget(const PREDICATELOCKTARGETTAG oldtargettag,
SHM_QUEUE *predlocktargetlink;
PREDICATELOCK *nextpredlock;
PREDICATELOCK *newpredlock;
+ SerCommitSeqNo oldCommitSeqNo = oldpredlock->commitSeqNo;
predlocktargetlink = &(oldpredlock->targetLink);
nextpredlock = (PREDICATELOCK *)
@@ -2544,8 +2545,17 @@ TransferPredicateLocksToNewTarget(const PREDICATELOCKTARGETTAG oldtargettag,
&(newpredlock->targetLink));
SHMQueueInsertBefore(&(newpredlocktag.myXact->predicateLocks),
&(newpredlock->xactLink));
-newpredlock->commitSeqNo = InvalidSerCommitSeqNo;
+newpredlock->commitSeqNo = oldCommitSeqNo;
}
+ else
+ {
+if (newpredlock->commitSeqNo < oldCommitSeqNo)
+ newpredlock->commitSeqNo = oldCommitSeqNo;
+ }
+
+ Assert(newpredlock->commitSeqNo != 0);
+ Assert((newpredlock->commitSeqNo == InvalidSerCommitSeqNo)
+ || (newpredlock->tag.myXact == OldCommittedSxact));
oldpredlock = nextpredlock;
}
@@ -3130,6 +3140,8 @@ ClearOldPredicateLocks(void)
offsetof(PREDICATELOCK, xactLink));
LWLockAcquire(SerializableXactHashLock, LW_SHARED);
+ Assert(predlock->commitSeqNo != 0);
+ Assert(predlock->commitSeqNo != InvalidSerCommitSeqNo);
canDoPartialCleanup = (predlock->commitSeqNo <= PredXact->CanPartialClearThrough);
LWLockRelease(SerializableXactHashLock);
@@ -3254,6 +3266,8 @@ ReleaseOneSerializableXact(SERIALIZABLEXACT *sxact, bool partial,
errhint("You might need to increase max_pred_locks_per_transaction.")));
if (found)
{
+Assert(predlock->commitSeqNo != 0);
+Assert(predlock->commitSeqNo != InvalidSerCommitSeqNo);
if (predlock->commitSeqNo < sxact->commitSeqNo)
predlock->commitSeqNo = sxact->commitSeqNo;
}
--
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: Support comments on FOREIGN DATA WRAPPER and SERVER objects.
2011/4/2 Robert Haas : > On Fri, Apr 1, 2011 at 11:57 AM, Thom Brown wrote: >> Should we also have support for comments on user mappings? > > Oh, bugger. Yeah, probably. I'd work on this, if taking some days is OK. Regards, -- Shigeru Hanada -- 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] Process local hint bit cache
Merlin Moncure writes: > On Thu, Mar 31, 2011 at 5:38 PM, Merlin Moncure wrote: >> working on exanding the cache to # xid > 1. > patch attached. this is essentially my original idea except it's > injected directly in to tqual.c as a kind of a expansion of the 'last > seen' concept. Because the cache loops are inlined and very tight (4 > int compares), it's actually cheaper than jumping out of line into > clog to test this_int = that_int; This seems like a mess. Why is the criterion for caching commit states different from whether the hint bit can be set? And why are you cluttering tqual.c with it? Based on the initial description I'd expected to find this enlarging the single-entry cache in transam.c to have multiple entries. 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] wal_buffers = -1 and SIGHUP
Robert Haas writes:
> On Thu, Mar 31, 2011 at 8:38 AM, Bernd Helmle wrote:
>> This might be nitpicking (or i'm currently missing something), but i
>> recognized that setting wal_buffers = -1 always triggers the following on
>> reload, even if nothing to wal_buffers had changed:
>>
>> $ pg_ctl reload
>> LOG: received SIGHUP, reloading configuration files
>> LOG: parameter "wal_buffers" cannot be changed without restarting the
>> server
>>
>> This only happens when you have wal_buffers set to -1.
> This is a bug. The root cause is that, on startup, we do this:
> if (xbuffers != XLOGbuffers)
> {
> snprintf(buf, sizeof(buf), "%d", xbuffers);
> SetConfigOption("wal_buffers", buf, PGC_POSTMASTER,
> PGC_S_OVERRIDE);
> }
> I had intended to commit Greg's patch with a show hook and an assign
> hook and the calculated value stored separately from the GUC variable,
> which I believe would avoid this is a problem, but Tom thought this
> way was better. Unfortunately, my knowledge of the GUC machinery is
> insufficient to think of a way to avoid it, other than the one Tom
> already rejected.
Mph ... I think this shows the error of my thinking :-(. We at least
need an assign hook here. Will fix, since it was my oversight to begin
with.
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] psql 9.1 alpha5: connection pointer is NULL
Joseph Adams writes: > I couldn't reproduce this (using upstream source on Ubuntu). However, > I did find a little bug in libpq causing the connection handle to > become NULL in the event of an option parsing error. This bug has > been around since release 9.0.0, and may be unrelated to the problem. Yeah, that's clearly a bug --- fix committed, thanks for the patch! It could explain Devrim's report if the parameters passed by psql had some problem that was detectable by conninfo_array_parse(). That seems a bit unlikely, but I did think of one possibility: if Devrim was testing 9.1 psql with a 9.0 libpq (perhaps due to an rpath issue) then 9.0 libpq would spit up on client_encoding, which wasn't a legal connection parameter in 9.0. 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] Bug in autovacuum.c?
Robert Haas writes: > On Fri, Apr 1, 2011 at 5:48 PM, Bruce Momjian wrote: >> Agreed it is not worth it but I think we should at least C comment >> something. I think at a minimum we should set it to >> FirstNormalTransactionId. > I think you should leave it well enough alone. Yes. The point of the existing coding is to ensure that we don't overestimate the table age at which vacuums should be forced. Bruce's proposed change would move the inaccuracy in the wrong direction, and thus cause some cases to not force autovac though an exact calculation would have done so. It's not worth trying to be exactly correct here, but I don't think that we want to err in that direction. If we had a symbol for the max normal XID, we could instead code like this: if (xidForceLimit < FirstNormalTransactionId) xidForceLimit = LastNormalTransactionId; But AFAIR we don't, and I don't especially want to introduce one, because people might be misled by it. As you mentioned earlier, the XID space is circular so there isn't really a "last" XID. 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] Another swing at JSON
Tom Lane writes: > So, I'm interested in trying to improve this, but it looks like a > research project from here. True: I don't have a baked solution that we would just need to apply. The simplest idea I can think of is forcing make install before to build contribs so that PGXS works “normally”. We would have to clarify the point somewhere visible in the docs though, so that we can say to people building extensions to mimic contrib/ except for the in-tree old-style Makefile support. In fact, adding some comments about that in all the contrib extension Makefiles might be our best shot at it, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Another swing at JSON
Tom Lane writes: > Dimitri Fontaine writes: >> This and removing module_pathname in the control files to just use >> $libdir/contrib in the .sql files. That would set a better example to >> people who want to make their own extensions, as the general case is >> that those will not get into contrib. > > My original intention when I started working with the extensions patch > had in fact been to do what you suggest above, but I was convinced not > to. I don't think we should reverse that decision at the last minute. Ok, I'll trust you on that. So the other way to ease compatibility would be to keep the .sql.in to .sql Makefile rule in pgxs.mk but make it just to a copy in 9.1. Then the same old script will just continue working as soon as you edit the module_pathname in the control file. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et 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] Process local hint bit cache
Merlin Moncure writes: > On Wed, Mar 30, 2011 at 2:35 PM, Merlin Moncure wrote: >> btw I haven't forgotten your idea to move TransactionIdInProgress >> Down. I think this is a good idea, and will experiment with it pre and >> post cache. The reason it's done in that order is to avoid race conditions. If you change the order you will get wrong behavior if the other transaction ends between the TransactionIdDidCommit and the TransactionIdInProgress tests. I suppose you could recheck TransactionIdDidCommit a second time, but that hardly seems likely to result in performance gains. > aside: > Moving TransactionIdInProgress below TransactionIdDidCommit can help > in once sense: TransactionIdDidCommit grabs the XidStatus but discards > the knowledge if the transaction is known aborted. Doesn't the single-entry cache help with that? 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] 9.0.3 SIGFAULT on FreeBSD with dtrace
Luca Ferrari writes: > I'm trying to compile PostgreSQL 9.0.3 on FreeBSD 8.1-stable, and I can make > it working if I compile without dtrace. However when I compile with --enable- > dtrace I'm unable to use the cluster and even initdb. You probably need to take that up with some FreeBSD dtrace hackers. It's possible that we need to adjust PG's dtrace code to support the FreeBSD implementation, but if so we'd need advice from an expert on what needs to be changed. 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] Another swing at JSON
Dimitri Fontaine writes: > This and removing module_pathname in the control files to just use > $libdir/contrib in the .sql files. That would set a better example to > people who want to make their own extensions, as the general case is > that those will not get into contrib. I'm not sure it's a better example. We considered doing that before, and decided not to on the grounds that using MODULE_PATHNAME avoids hard-wiring the name of the shared library into the SQL scripts. Also, IIRC, there are a couple of contrib modules where there's an actual problem in doing it like that; though I'm unable to recall details as I'm fighting a head cold at the moment. My original intention when I started working with the extensions patch had in fact been to do what you suggest above, but I was convinced not to. I don't think we should reverse that decision at the last minute. 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] psql 9.1 alpha5: connection pointer is NULL
2011/4/2 Devrim GÜNDÜZ :
>
> I'm getting the following message after upgrading to Alpha5 on my Fedora
> 14 box:
>
> $ psql -p 5433
> psql: connection pointer is NULL
>
> which comes from libpq. Server is running, and I can connect it to via
> 9.0's psql.
>
> This is a regular RPM build. Am I doing something wrong, or?
I couldn't reproduce this (using upstream source on Ubuntu). However,
I did find a little bug in libpq causing the connection handle to
become NULL in the event of an option parsing error. This bug has
been around since release 9.0.0, and may be unrelated to the problem.
Joey Adams
From 5bcf99b6481dd1393525ee804eeae6a04999d86d Mon Sep 17 00:00:00 2001
From: Joey Adams
Date: Sat, 2 Apr 2011 14:27:22 -0400
Subject: [PATCH] Fixed "return false;" in PQconnectStartParams, which returns a PGconn *
---
src/interfaces/libpq/fe-connect.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index a4959ee..aa24c37 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -491,7 +491,7 @@ PQconnectStartParams(const char **keywords,
{
conn->status = CONNECTION_BAD;
/* errorMessage is already set */
- return false;
+ return conn;
}
/*
--
1.7.1
--
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] Another swing at JSON
Andrew Dunstan writes: > On 03/30/2011 12:29 PM, Dimitri Fontaine wrote: >> Andrew Dunstan writes: >>> I think we're pretty much down to only fixing bugs now, for 9.1, and this >>> isn't a bug, however inconvenient it might be. >> It's not just inconvenient, it's setting a bad example for people to >> work on their own extensions. It's more than unfortunate. I will >> prepare a doc patch, but copying from contrib is the usual way to go >> creating your own extension, right? > None of that makes it a bug. Possibly more to the point, we don't even have a design sketch for a better solution; and we are *certainly* past the point where blue-sky stuff ought to be going into 9.1. The reason it seems (to me) nontrivial to change is this: the PGXS build method assumes that the correct pg_config can be found in your PATH. That is pretty much guaranteed to not be the case during a in-tree build. Even if we modified the PATH to include wherever pg_config is hiding, the information it puts out about where to look for include and library files would be wrong. Another small problem with depending on pg_config during an in-tree build is that it would completely break cross-compile builds. (Maybe those are in bad shape already, I'm not sure. But configure for example is still going out of its way to support them.) So, I'm interested in trying to improve this, but it looks like a research project from here. 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] Transforming IN (...) to ORs, volatility
Heikki Linnakangas writes: > We sometimes transform IN-clauses to a list of ORs: > postgres=# explain SELECT * FROM foo WHERE a IN (b, c); >QUERY PLAN > -- > Seq Scan on foo (cost=0.00..39.10 rows=19 width=12) > Filter: ((a = b) OR (a = c)) > (2 rows) > But what if you replace "a" with a volatile function? It doesn't seem > legal to do that transformation in that case, but we do it: This is the fault of transformAExprIn(). But please let's *not* fix this by adding volatility to the set of heuristics used there. Looking at this again, it seems to me that most of the problem with this code is that we're trying to make optimization decisions in the parser. I think what we ought to do is have the parser emit a full-fledged InExpr node type (with semantics rather like CaseExpr) and then teach the planner to optimize that to something else when it seems safe/prudent to do so. One nontrivial advantage of that is that rules/views containing IN constructs would start to reverse-parse in the same fashion, instead of introducing weird substitute expressions. 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
[HACKERS] Re: [COMMITTERS] pgsql: Escape greater than and less than characters in docs.
On Sat, Apr 02, 2011 at 02:09:29PM +, Heikki Linnakangas wrote: > Escape greater than and less than characters in docs. Should things like this (< and > with space around them, I'm thinking) be part of make maintainer-check? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Finding a flow when query is fired in postgresql
i edited postgreql file so that i can check the flow of .c files invoked when we fire a query but only what i am getting is just parse trees ,rewritten trees and plan but no names of .c files invoked... plz help me asap ...its very important for me. what additional changes are required to see the names of .c files when query is fired.. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Finding-a-flow-when-query-is-fired-in-postgresql-tp4277478p4277478.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- 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] [DOCS] fixed doc bug in sepgsql.sgml
On 02.04.2011 10:24, Susanne Ebrecht wrote: Hello, by accident we recognised that the author of sepgsql.sgml used < and > instead of < and > I just fixed it and here is the patch. Committed, and I also fixed one case of that in pg_basebackup docs. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- 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] corner case about replication and shutdown
On Fri, Apr 1, 2011 at 11:11 PM, Robert Haas wrote: > On Thu, Mar 31, 2011 at 11:12 PM, Fujii Masao wrote: >> Another simple fix is to make walsender send SIGUSR1 to postmaster >> so that it calls PostmasterStateMachine() in sigusr1_handler(), when it >> marks itself as walsender. The attached patch does this. Thought? > > That looks OK to me. Have you tested it? Yes. I added the sleep just before MarkPostmasterChildWalSender() in walsender.c, compiled, started replication, and then requested smart shutdown as soon as walsender was forked (i.e., during the sleep). Without the patch, the server got stuck infinitely. With the patch, smart shutdown worked as expected. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] psql 9.1 alpha5: connection pointer is NULL
I'm getting the following message after upgrading to Alpha5 on my Fedora 14 box: $ psql -p 5433 psql: connection pointer is NULL which comes from libpq. Server is running, and I can connect it to via 9.0's psql. This is a regular RPM build. Am I doing something wrong, or? Regards, -- Devrim GÜNDÜZ Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer Community: devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr http://www.gunduz.org Twitter: http://twitter.com/devrimgunduz signature.asc Description: This is a digitally signed message part
Re: [HACKERS] [DOCS] found a very confusing and maybe outdated sentence
On 31.03.2011 18:13, Robert Haas wrote: On Tue, Mar 29, 2011 at 3:07 PM, Susanne Ebrecht wrote: Hello, It is in start.sgml. You can see it here: http://www.postgresql.org/docs/9.0/static/tutorial-accessdb.html The last two sentences on the page: " If PostgreSQL is installed correctly you can also type man psql at the operating system shell prompt to see the documentation. In this tutorial we will not use these features explicitly, but you can use them yourself when it is helpful." Isn't that totally outdated? Is "man" really working on Windows? Also the sentence says that the whole product isn't correct installed just because docs aren't installed. Which also isn't really true. Honesty, I just would like to drop the whole sentences. What do you think? +1 for deleting the entire sentence. Here is the patch for removing the whole sentence. Susanne -- Susanne Ebrecht - 2ndQuadrant PostgreSQL Development, 24x7 Support, Training and Services www.2ndQuadrant.com diff --git a/doc/src/sgml/start.sgml b/doc/src/sgml/start.sgml index 4275bf8..0bfc25e 100644 --- a/doc/src/sgml/start.sgml +++ b/doc/src/sgml/start.sgml @@ -403,11 +403,7 @@ mydb=# command shell. (For more internal commands, type \? at the psql prompt.) The full capabilities of psql are documented in -. If PostgreSQL is -installed correctly you can also type man psql -at the operating system shell prompt to see the documentation. In -this tutorial we will not use these features explicitly, but you -can use them yourself when it is helpful. +. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] fixed doc bug in sepgsql.sgml
Hello, by accident we recognised that the author of sepgsql.sgml used < and > instead of < and > I just fixed it and here is the patch. Susanne -- Susanne Ebrecht - 2ndQuadrant PostgreSQL Development, 24x7 Support, Training and Services www.2ndQuadrant.com diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml index 21b46de..589fe79 100644 --- a/doc/src/sgml/sepgsql.sgml +++ b/doc/src/sgml/sepgsql.sgml @@ -105,7 +105,7 @@ $ initdb $ vi $PGDATA/postgresql.conf $ for DBNAME in template0 template1 postgres; do postgres --single -F -O -c exit_on_error=true $DBNAME \ - < /usr/local/pgsql/share/contrib/sepgsql.sql > /dev/null + < /usr/local/pgsql/share/contrib/sepgsql.sql > /dev/null done -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
