Re: [HACKERS] Report: removing the inconsistencies in our CVS->git conversion
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> the tarball was actually made. In particular, the tags REL6_5, REL7_1, > >> and REL7_1_2 don't match the tarballs they ought to. I don't have a whole > >> lot of faith in some of the other early tags either, because we don't seem > >> to have an archived tarball to compare them to. > > > I believe I have those on a CDROM here. > > If you can recover any of the releases that aren't on ftp-archive, > please send me copies. Sure. I have a copy of our ftp site /pub as of 6.3 and have put it online: http://momjian.us/expire/pgsql_ftp_6.3/ Unfortunately I don't see anything there that isn't already here: ftp://ftp-archives.postgresql.org/pub/source/ but let me know if you find something new. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] update on global temporary and unlogged tables
On Mon, Sep 13, 2010 at 9:06 PM, Rob Wultsch wrote: > On Mon, Sep 6, 2010 at 7:55 PM, Robert Haas wrote: >> >> 3. With respect to unlogged tables, the major obstacle seems to be >> figuring out a way for these to get automatically truncated at startup >> time. > > (please forgive what is probably a stupid question) > By truncate do mean reduce the table to a very small number (or zero) number > of pages? Is there a case to be made for instead somehow marking all pages > as available for reuse? Deallocating and reallocating space can be > expensive. I think it's probably actually cheaper to truncate them, but since it only happens at startup time it's probably not worth worrying about -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] patch: Add JSON datatype to PostgreSQL (GSoC, WIP)
On 09/13/2010 09:30 PM, Itagaki Takahiro wrote: Hi, Anyone working on JSON datatype? If no, I'm going to submit simplified version of JSON datatype patch. What's the state of the GSOC project? 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] Reducing walreceiver latency with a latch
On Mon, Sep 13, 2010 at 9:13 PM, Heikki Linnakangas wrote: > Here's an updated patch with those bugs fixed. Great! + /* +* Walreceiver sets this latch every time new WAL has been received and +* fsync'd to disk, allowing startup process to wait for new WAL to +* arrive. +*/ + Latch receivedLatch; I think that this latch should be available for other than walreceiver - startup process communication. For example, backend - startup process communication, which can be used for requesting a failover via SQL function by users in the future. What about putting the latch in XLogCtl instead of WalRcv and calling OwnLatch at the beginning of the startup process instead of RequestXLogStreaming? 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
Re: [HACKERS] patch: Add JSON datatype to PostgreSQL (GSoC, WIP)
Hi, Anyone working on JSON datatype? If no, I'm going to submit simplified version of JSON datatype patch. On Wed, Aug 25, 2010 at 2:34 PM, Itagaki Takahiro wrote: > On Fri, Aug 13, 2010 at 7:33 PM, Joseph Adams > wrote: >> Updated patch: the JSON code has all been moved into core, so this >> patch is now for a built-in data type. > > I think the patch can be split into two pieces: > 1. Basic I/O support for JSON type (in/out/validate) > 2. JSONPath support and functions for partial node management > > It is better to submit only 1 at first. Of course we should consider > about JSONPath before deciding the internal representation of JSON, > but separated patches can be easily reviewed. -- Itagaki Takahiro -- 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] update on global temporary and unlogged tables
On Mon, Sep 6, 2010 at 7:55 PM, Robert Haas wrote: > 3. With respect to unlogged tables, the major obstacle seems to be > figuring out a way for these to get automatically truncated at startup > time. > (please forgive what is probably a stupid question) By truncate do mean reduce the table to a very small number (or zero) number of pages? Is there a case to be made for instead somehow marking all pages as available for reuse? Deallocating and reallocating space can be expensive. -- Rob Wultsch wult...@gmail.com
Re: [HACKERS] pg_ctl emits strange warning message
On Tue, Sep 14, 2010 at 12:10 AM, Heikki Linnakangas wrote: > Yet another idea: Check in pg_ctl if recovery.conf is also present. If it > is, assume we're in recovery and don't print that warning. That would be > dead simple. I would even consider backpatching it to 9.0, this can be > regarded as a bug, albeit a minor one. Yeah, it's very simple. Attached patch does that. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center warning_during_shutdown_v2.patch 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
Re: [HACKERS] top-level DML under CTEs
2010/9/14 Merlin Moncure : > On Mon, Sep 13, 2010 at 9:20 AM, Robert Haas wrote: >> On Mon, Sep 13, 2010 at 9:15 AM, Hitoshi Harada wrote: >>> The patch attached is based on the one rejected at the last CF for 9.0 >>> last year. >>> >>> http://archives.postgresql.org/message-id/16303.1266023...@sss.pgh.pa.us >>> >>> This patch implements the feature that allows top-level DMLs under CTE >>> WITH clause. For example: >>> >>> WITH t AS (SELECT * FROM x) >>> UPDATE y SET val = t.val FROM t >>> WHERE y.key = t.key; >>> >>> This feature is part of writeable CTEs proposed by David Fetter originally. >> >> Thanks for pursuing this. I think this will be a useful feature if we >> can get it committed, and plus David Fetter will be very, very happy. >> :-) > > Just to be clear, the attached patch is missing the part of the wCTE Yes, the main part of wCTE that allows DMLs in WITH is still under Marko Tikkaja. To work parallel, we split the tasks into pieces. Regards, -- Hitoshi Harada -- 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] pg_ctl emits strange warning message
On Tue, Sep 14, 2010 at 12:35 AM, Heikki Linnakangas wrote: > Hmm, looking at this more closely, I'm a bit confused. We already rename > away backup_label at the beginning of recovery. Under what circumstances do > we have a problem? This starts to seem like a complete non-issue to me. A checkpoint record is read before backup_label file is renamed, so the problem happens if shutdown is requested while the standby server is waiting for the WAL containing a checkpoint record to arrive from the master. This happened some times in my box when testing HS/SR. To avoid the problem, we should rename backup_label before reading any WAL record? 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
Re: [HACKERS] Report: removing the inconsistencies in our CVS->git conversion
Bruce Momjian writes: > Tom Lane wrote: >> the tarball was actually made. In particular, the tags REL6_5, REL7_1, >> and REL7_1_2 don't match the tarballs they ought to. I don't have a whole >> lot of faith in some of the other early tags either, because we don't seem >> to have an archived tarball to compare them to. > I believe I have those on a CDROM here. If you can recover any of the releases that aren't on ftp-archive, please send me copies. 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] Report: removing the inconsistencies in our CVS->git conversion
Tom Lane wrote: > the tarball was actually made. In particular, the tags REL6_5, REL7_1, > and REL7_1_2 don't match the tarballs they ought to. I don't have a whole > lot of faith in some of the other early tags either, because we don't seem > to have an archived tarball to compare them to. I believe I have those on a CDROM here. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] "serializable" in comments and names
>Joe Conway wrote: > Committed. Thanks! I'm pulling together a new version of the main patch, and it is almost 300 lines shorter and touches five fewer files than the last version because this went in. It should be easier for people to scan to understand the substance of the changes without the noop changes interleaved with "real" ones. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Update PostgreSQL shared memory usage table for 9.0?
Can someone update the PostgreSQL shared memory usage table for 9.0? http://www.postgresql.org/docs/9.0/static/kernel-resources.html#SYSVIPC Right now it says "Approximate shared memory bytes required (as of 8.3)". -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] Report: removing the inconsistencies in our CVS->git conversion
On Mon, Sep 13, 2010 at 21:28, Tom Lane wrote: > I wrote: >> return bool(re.match( >> r'file .* was added on branch .* on ' >> r'\d{4}\-\d{2}\-\d{2} \d{2}\:\d{2}\:\d{2}( [\+\-]\d{4})?' >> '\n$', >> log_msg, >> )) > >> So it looks like I have to make the dead revisions' log messages match >> that regexp. Off to make another try. > > It works! Now I don't see either the manufactured commits or the > patched-in deletions. > > I had not previously bothered to patch the places where a file was added > on the branch immediately after being added on the main, but now it > seems worth doing. That will get us down to a *very* small number of > manufactured commits in the final version. That's awesome! Thanks so much for doing this. I've come to realize I know far too little about *cvs* to work on those things myself :S -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Report: removing the inconsistencies in our CVS->git conversion
I wrote: > return bool(re.match( > r'file .* was added on branch .* on ' > r'\d{4}\-\d{2}\-\d{2} \d{2}\:\d{2}\:\d{2}( [\+\-]\d{4})?' > '\n$', > log_msg, > )) > So it looks like I have to make the dead revisions' log messages match > that regexp. Off to make another try. It works! Now I don't see either the manufactured commits or the patched-in deletions. I had not previously bothered to patch the places where a file was added on the branch immediately after being added on the main, but now it seems worth doing. That will get us down to a *very* small number of manufactured commits in the final version. 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] top-level DML under CTEs
On Mon, Sep 13, 2010 at 2:43 PM, Merlin Moncure wrote: > Just to be clear, the attached patch is missing the part of the wCTE > that allows queries of the form: > WITH foo AS (DELETE * FROM bar RETURNING *) Understood. > IOW, your CTE query has to be a select. This is still highly useful > however. Agreed. > The patch itself looks very clean after a quick glance > (which is all I can offer ATM unfortunately). Yeah, I haven't had a chance to look at it either, just wanted to send props. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] top-level DML under CTEs
On Mon, Sep 13, 2010 at 9:20 AM, Robert Haas wrote: > On Mon, Sep 13, 2010 at 9:15 AM, Hitoshi Harada wrote: >> The patch attached is based on the one rejected at the last CF for 9.0 >> last year. >> >> http://archives.postgresql.org/message-id/16303.1266023...@sss.pgh.pa.us >> >> This patch implements the feature that allows top-level DMLs under CTE >> WITH clause. For example: >> >> WITH t AS (SELECT * FROM x) >> UPDATE y SET val = t.val FROM t >> WHERE y.key = t.key; >> >> This feature is part of writeable CTEs proposed by David Fetter originally. > > Thanks for pursuing this. I think this will be a useful feature if we > can get it committed, and plus David Fetter will be very, very happy. > :-) Just to be clear, the attached patch is missing the part of the wCTE that allows queries of the form: WITH foo AS (DELETE * FROM bar RETURNING *) IOW, your CTE query has to be a select. This is still highly useful however. The patch itself looks very clean after a quick glance (which is all I can offer ATM unfortunately). merlin -- 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] Policy decisions and cosmetic issues remaining for the git conversion
Magnus Hagander writes: > Yeah, let's not touch the CVS side, but definitely +1 for dropping > them from git (in fact, my script does this automatically if I just > let it run through all the steps, which I've repeatedly not done which > is why they've sometimes shown up and sometimes not in the ones I've > pushed) Could we see your full script and options file? I'm busy running test conversions myself, and would like to be sure that I'm on the same page as you are. BTW, the conversion I'm getting at the moment (using the fixup script I posted last night) does not have the weird "unlabeled" branches that were complained of earlier. I think that nuking the WIN32_DEV derived files probably was what stopped 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] Policy decisions and cosmetic issues remaining for the git conversion
On Mon, Sep 13, 2010 at 18:31, Tom Lane wrote: > This is an attempt to sum up the open issues remaining before we can > make another try at converting our source code to git. > * The REL8_0_0 branch needs to be downgraded to a tag, as previously > discussed. Yeah, and that's easily done. > * There are several things we should do to make the manufactured commits > less ugly, assuming we can't get rid of them entirely: > ** Blame them on a nonexistent committer, probably "cvs2git" itself. I've set the script onthe prod box up to blame it on cvs2git (same email address as the pgsql user, but different name) > ** If we do that, I'm inclined to also blame the inserted dead .0 > revisions on cvs2git. Right now I copied-and-pasted the committer info > from the mainline commit they inherit from, which is not very relevant. +1 > ** Make the manufactured-commit log messages say cvs2git not cvs2svn. Done in my options file - it's just a couple of settings. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Policy decisions and cosmetic issues remaining for the git conversion
On Mon, Sep 13, 2010 at 19:14, Robert Haas wrote: > On Mon, Sep 13, 2010 at 12:50 PM, Alvaro Herrera > wrote: >> Excerpts from Tom Lane's message of lun sep 13 12:31:53 -0400 2010: >> >>> * As I noted previously, up till about 2003 we were quite haphazard about >>> applying CVS tags to identify the points where releases were made. Should >>> we try to clean that up? I think there is a stronger case for moving the >>> three existing misleading tags than for creating new tags matching the >>> releases that have none. Maybe nobody will ever care about any of them, >>> but if we are trying to create a good historical record it might be >>> appropriate to do it now while we have the information in mind. >> >> +1 on both -- fixing the broken tags, and creating the missing tags, >> particularly since you already seem to have found out the necessary >> dates for the missing tags. > > +1 from me, too. I don't agree with statements upthread that this > will be "easier" to do in git. I think we should fix the CVS history. > The git conversion is a one-time event. Once it's done, history is > set in stone. We don't want to set the wrong thing in stone. Yeah, +1 on fixing it there if it needs fixing. As noted downstream, we need to make sure it's *fixable* anyway, and that's the larger part of the work I imagine. >>> * There are a number of partial tags (tags applied to just a subset of >>> files) in the CVS repository: "MANUAL_1_0" and "SUPPORT" seem to have been >>> applied to only documentation-related files, and "creation" and >>> "Release-1-6-0" were applied only to src/interfaces/perl5/. I find the >>> latter two particularly misleading since they have nothing to do with >>> either creation of the whole project or a "release 1.6" of the whole >>> project. These partial tags don't translate very well to git, either. >>> I'm inclined to propose dropping all four. >> >> +1 on dropping these. > > Yeah. I would leave these in CVS (why not?) but drop them from the > git conversion, which is easily done. Yeah, let's not touch the CVS side, but definitely +1 for dropping them from git (in fact, my script does this automatically if I just let it run through all the steps, which I've repeatedly not done which is why they've sometimes shown up and sometimes not in the ones I've pushed) >>> * There are a couple of manufactured commits that we could just delete, >>> I think: the ones to create the Release_2_0 and Release_2_0_0 tags. The >>> tags should be reapplied to the chronologically preceding mainline commits >>> instead. This is just cosmetic but we may as well do it. I still think >>> there's a cvs2git bug underlying those. >> >> +1 > > +1. +1. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.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] Policy decisions and cosmetic issues remaining for the git conversion
Robert Haas writes: > On Mon, Sep 13, 2010 at 1:21 PM, Tom Lane wrote: >> Well, the other side of that argument is that changing these things in >> the CVS repository will be overwriting the available evidence, in case >> any questions come up later. On the git side, applying the tag to the >> appropriate commit is an easy --- and easily changeable --- thing, isn't >> it? > You can apply the tag to any commit that you want easily enough, but > you can't change even the least detail of the commit contents. So if > it turns out that there's no commit that exactly matches the desired > tag contents, then things get sticky. That's why we need to make sure > that the reconstructed series of commits exactly matches what really > happened as closely as possible the first time through. If it > doesn't, we're pretty hosed. I've already found the commits on the CVS side that I think ought to get the tags --- that's what that "matches" file is about. If cvs2git can't be trusted to duplicate those tree states then we have bigger problems. Of course, now that I've got the tarballs I will be rechecking them against git checkouts as part of the acceptance tests for the final conversion, but right now I don't foresee a problem 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] Policy decisions and cosmetic issues remaining for the git conversion
On Mon, Sep 13, 2010 at 1:21 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Sep 13, 2010 at 12:50 PM, Alvaro Herrera >> wrote: >>> +1 on both -- fixing the broken tags, and creating the missing tags, >>> particularly since you already seem to have found out the necessary >>> dates for the missing tags. > >> +1 from me, too. I don't agree with statements upthread that this >> will be "easier" to do in git. I think we should fix the CVS history. >> The git conversion is a one-time event. Once it's done, history is >> set in stone. We don't want to set the wrong thing in stone. > > Well, the other side of that argument is that changing these things in > the CVS repository will be overwriting the available evidence, in case > any questions come up later. On the git side, applying the tag to the > appropriate commit is an easy --- and easily changeable --- thing, isn't > it? You can apply the tag to any commit that you want easily enough, but you can't change even the least detail of the commit contents. So if it turns out that there's no commit that exactly matches the desired tag contents, then things get sticky. That's why we need to make sure that the reconstructed series of commits exactly matches what really happened as closely as possible the first time through. If it doesn't, we're pretty hosed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
On 13/09/10 20:43, Jeff Davis wrote: On Mon, 2010-09-13 at 09:10 +0300, Heikki Linnakangas wrote: but we should be consistent and document that: (a) it shouldn't happen (b) that it's just a sanity check and we're ignoring the race Would this be sufficient? --- a/src/backend/port/unix_latch.c +++ b/src/backend/port/unix_latch.c @@ -156,6 +156,7 @@ OwnLatch(volatile Latch *latch) if (selfpipe_readfd == -1) initSelfPipe(); + /* sanity check */ if (latch->owner_pid != 0) elog(ERROR, "latch already owned"); latch->owner_pid = MyProcPid; Or you want to suggest something better? Perfect. I was just slightly confused reading it the first time, and I think that would have cleared it up. Ok, added that. -- 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: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
On Mon, 2010-09-13 at 09:10 +0300, Heikki Linnakangas wrote: > > but we should be consistent and document that: > > (a) it shouldn't happen > > (b) that it's just a sanity check and we're ignoring the race > > Would this be sufficient? > > --- a/src/backend/port/unix_latch.c > +++ b/src/backend/port/unix_latch.c > @@ -156,6 +156,7 @@ OwnLatch(volatile Latch *latch) > if (selfpipe_readfd == -1) > initSelfPipe(); > > + /* sanity check */ > if (latch->owner_pid != 0) > elog(ERROR, "latch already owned"); > latch->owner_pid = MyProcPid; > > Or you want to suggest something better? Perfect. I was just slightly confused reading it the first time, and I think that would have cleared it up. Thanks, Jeff Davis -- 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] Report: removing the inconsistencies in our CVS->git conversion
I wrote: > I'm a bit disappointed by the fact that we get either of these. I had > gathered from Max's comments that the dead-revision-at-the-base-of-the- > branch trick is considered standard in newer CVS versions, and so I'd > hoped that cvs2git would understand the construct and not generate > either of these commits. Possibly the hacked-up revisions I inserted > are enough different from the regular kind to confuse it. Hah: a bit of digging in the cvs2svn sources found this: def _is_unneeded_initial_branch_delete(self, lod_items, metadata_db): """Return True iff the initial revision in LOD_ITEMS can be deleted.""" if not lod_items.cvs_revisions: return False cvs_revision = lod_items.cvs_revisions[0] if cvs_revision.ntdbr: return False if not isinstance(cvs_revision, CVSRevisionAbsent): return False if cvs_revision.branch_ids: return False log_msg = metadata_db[cvs_revision.metadata_id].log_msg return bool(re.match( r'file .* was added on branch .* on ' r'\d{4}\-\d{2}\-\d{2} \d{2}\:\d{2}\:\d{2}( [\+\-]\d{4})?' '\n$', log_msg, )) So it looks like I have to make the dead revisions' log messages match that regexp. Off to make another try. 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] Policy decisions and cosmetic issues remaining for the git conversion
Robert Haas writes: > On Mon, Sep 13, 2010 at 12:50 PM, Alvaro Herrera > wrote: >> +1 on both -- fixing the broken tags, and creating the missing tags, >> particularly since you already seem to have found out the necessary >> dates for the missing tags. > +1 from me, too. I don't agree with statements upthread that this > will be "easier" to do in git. I think we should fix the CVS history. > The git conversion is a one-time event. Once it's done, history is > set in stone. We don't want to set the wrong thing in stone. Well, the other side of that argument is that changing these things in the CVS repository will be overwriting the available evidence, in case any questions come up later. On the git side, applying the tag to the appropriate commit is an easy --- and easily changeable --- thing, isn't 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] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
On 09/13/2010 06:43 PM, Greg Smith wrote: Thom Brown wrote: I thought sysbench was designed for MySQL benchmarks. How new is the PostgreSQL driver? Is it stable yet? It's been out there for years; the FreeBSD 7.0 development used it extensively on MySQL and PostgreSQL to track kernel performance on both databases back in 2007: http://people.freebsd.org/~kris/scaling/7.0%20Preview.pdf I don't think "stable" applies here just based on code age though, given how infrequent updates to the sysbench code are and how little QA is put into them. They pushed out two updates in 2009, 0.4.11 and 0.4.12, but all they did for me was break basic compilation on multiple platforms. I still use 0.4.10 as the last version that seems to work without makefile surgery on both RedHat and Ubuntu. The last time I tried it, the read-only OLTP implementation worked fine, but the one that wrote instead was prone to deadlocks in PostgreSQL. yeah the read-only part works quite well(the other ones not so much) and it was much faster than pgbench in older pg release - I have not looked yet if the new threaded in 9.0 implementation fixes that issue. Stefan -- 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] Report: removing the inconsistencies in our CVS->git conversion
On Mon, Sep 13, 2010 at 1:14 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Sep 13, 2010 at 11:48 AM, Tom Lane wrote: >>> Robert Haas writes: I wonder if we should consider fixing some or all of these things on the master CVS repository. I wouldn't be too eager to inject those fake .0 commits for fear of breakage, but moving tags to where they ought to have been all along seems like it might be a good thing to do independent of git. > >>> Yeah, that's something I was wondering too. Applying these fixes to the >>> master repository would also reduce the number of things we have to >>> remember to do during the final conversion. OTOH, there's that risk of >>> breaking something. > >> Hand-written patches that apply directly to the RCS files seem like >> they'd be a risk for breakage, but I don't see why moving tags around >> would be all that dangerous, especially in cases where you can do it >> by running 'cvs' itself rather than 'rcs'. That should just be >> routine stuff, no? > > Hrm, well, keep in mind that most of these problems were *created* by > careless use of "cvs tag". At the moment I'm leaning towards the idea > that we should leave the CVS repository as it is, rather than take any > risk of making things worse. I think that I have never, and am never likely ever to, hear anyone describe you as careless. I feel pretty much 100% safe having you retag those releases to match the tarballs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Policy decisions and cosmetic issues remaining for the git conversion
On Mon, Sep 13, 2010 at 12:50 PM, Alvaro Herrera wrote: > Excerpts from Tom Lane's message of lun sep 13 12:31:53 -0400 2010: > >> * As I noted previously, up till about 2003 we were quite haphazard about >> applying CVS tags to identify the points where releases were made. Should >> we try to clean that up? I think there is a stronger case for moving the >> three existing misleading tags than for creating new tags matching the >> releases that have none. Maybe nobody will ever care about any of them, >> but if we are trying to create a good historical record it might be >> appropriate to do it now while we have the information in mind. > > +1 on both -- fixing the broken tags, and creating the missing tags, > particularly since you already seem to have found out the necessary > dates for the missing tags. +1 from me, too. I don't agree with statements upthread that this will be "easier" to do in git. I think we should fix the CVS history. The git conversion is a one-time event. Once it's done, history is set in stone. We don't want to set the wrong thing in stone. >> * There are a number of partial tags (tags applied to just a subset of >> files) in the CVS repository: "MANUAL_1_0" and "SUPPORT" seem to have been >> applied to only documentation-related files, and "creation" and >> "Release-1-6-0" were applied only to src/interfaces/perl5/. I find the >> latter two particularly misleading since they have nothing to do with >> either creation of the whole project or a "release 1.6" of the whole >> project. These partial tags don't translate very well to git, either. >> I'm inclined to propose dropping all four. > > +1 on dropping these. Yeah. I would leave these in CVS (why not?) but drop them from the git conversion, which is easily done. >> * There are a couple of manufactured commits that we could just delete, >> I think: the ones to create the Release_2_0 and Release_2_0_0 tags. The >> tags should be reapplied to the chronologically preceding mainline commits >> instead. This is just cosmetic but we may as well do it. I still think >> there's a cvs2git bug underlying those. > > +1 +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Report: removing the inconsistencies in our CVS->git conversion
Robert Haas writes: > On Mon, Sep 13, 2010 at 11:48 AM, Tom Lane wrote: >> Robert Haas writes: >>> I wonder if we should consider fixing some or all of these things on >>> the master CVS repository. I wouldn't be too eager to inject those >>> fake .0 commits for fear of breakage, but moving tags to where they >>> ought to have been all along seems like it might be a good thing to do >>> independent of git. >> Yeah, that's something I was wondering too. Applying these fixes to the >> master repository would also reduce the number of things we have to >> remember to do during the final conversion. OTOH, there's that risk of >> breaking something. > Hand-written patches that apply directly to the RCS files seem like > they'd be a risk for breakage, but I don't see why moving tags around > would be all that dangerous, especially in cases where you can do it > by running 'cvs' itself rather than 'rcs'. That should just be > routine stuff, no? Hrm, well, keep in mind that most of these problems were *created* by careless use of "cvs tag". At the moment I'm leaning towards the idea that we should leave the CVS repository as it is, rather than take any risk of making things worse. 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] Report: removing the inconsistencies in our CVS->git conversion
On Mon, Sep 13, 2010 at 11:48 AM, Tom Lane wrote: > Robert Haas writes: >> On Sun, Sep 12, 2010 at 11:03 PM, Tom Lane wrote: >>> Having completed that comparison, I then moved on to trying to get rid of >>> the discrepancies in the git conversion; particularly, trying to get rid >>> of the "manufactured commits". I didn't have much success in that for the >>> cases where the manufactured commit was caused by a back-branch file >>> addition. [...] We still have "manufactured" commits either >>> way, but they are just cosmetic so I guess we should live with them. > >> I'm not really following what the history looks like here. What are >> the contents (git show) of the manufactured commit? > > A typical example is > > commit 4d2ac8075a93c685dbbe920f4bac23288dd7cf11 > Author: PostgreSQL Daemon > Date: Tue Nov 22 18:17:36 2005 + > > This commit was manufactured by cvs2svn to create branch 'REL7_4_STABLE'. > > Cherrypick from master 2005-11-22 18:17:34 UTC Bruce Momjian > 'Re-run pgindent, fixing a problem where comment lines > after a blank': > src/port/unsetenv.c > > diff --git a/src/port/unsetenv.c b/src/port/unsetenv.c > new file mode 100644 > index 000..bdfb3f6 > --- /dev/null > +++ b/src/port/unsetenv.c > @@ -0,0 +1,56 @@ > + [ entire contents of unsetenv.c here ] > > In the cases where I inserted a dead .0 revision, this is followed by > something like > > commit a1bdd263ca8ff657365a97a560f6371f39295efc > Author: Bruce Momjian > Date: Tue Nov 22 18:17:37 2005 + > > Mark branch as deleted. If we have two commits one right after the other that cancel each other out, we might be able to write them both out of the history using git-filter-branch. But if Max or Michael can shed any light on why it's happening, that might lead to a simpler solution. >>> I also found numerous places where we'd been sloppy about placing tags. > >> I wonder if we should consider fixing some or all of these things on >> the master CVS repository. I wouldn't be too eager to inject those >> fake .0 commits for fear of breakage, but moving tags to where they >> ought to have been all along seems like it might be a good thing to do >> independent of git. > > Yeah, that's something I was wondering too. Applying these fixes to the > master repository would also reduce the number of things we have to > remember to do during the final conversion. OTOH, there's that risk of > breaking something. Hand-written patches that apply directly to the RCS files seem like they'd be a risk for breakage, but I don't see why moving tags around would be all that dangerous, especially in cases where you can do it by running 'cvs' itself rather than 'rcs'. That should just be routine stuff, no? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Policy decisions and cosmetic issues remaining for the git conversion
Excerpts from Tom Lane's message of lun sep 13 12:31:53 -0400 2010: > * As I noted previously, up till about 2003 we were quite haphazard about > applying CVS tags to identify the points where releases were made. Should > we try to clean that up? I think there is a stronger case for moving the > three existing misleading tags than for creating new tags matching the > releases that have none. Maybe nobody will ever care about any of them, > but if we are trying to create a good historical record it might be > appropriate to do it now while we have the information in mind. +1 on both -- fixing the broken tags, and creating the missing tags, particularly since you already seem to have found out the necessary dates for the missing tags. > * There are a number of partial tags (tags applied to just a subset of > files) in the CVS repository: "MANUAL_1_0" and "SUPPORT" seem to have been > applied to only documentation-related files, and "creation" and > "Release-1-6-0" were applied only to src/interfaces/perl5/. I find the > latter two particularly misleading since they have nothing to do with > either creation of the whole project or a "release 1.6" of the whole > project. These partial tags don't translate very well to git, either. > I'm inclined to propose dropping all four. +1 on dropping these. > * There are a couple of manufactured commits that we could just delete, > I think: the ones to create the Release_2_0 and Release_2_0_0 tags. The > tags should be reapplied to the chronologically preceding mainline commits > instead. This is just cosmetic but we may as well do it. I still think > there's a cvs2git bug underlying those. +1 -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. 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] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
Thom Brown wrote: I thought sysbench was designed for MySQL benchmarks. How new is the PostgreSQL driver? Is it stable yet? It's been out there for years; the FreeBSD 7.0 development used it extensively on MySQL and PostgreSQL to track kernel performance on both databases back in 2007: http://people.freebsd.org/~kris/scaling/7.0%20Preview.pdf I don't think "stable" applies here just based on code age though, given how infrequent updates to the sysbench code are and how little QA is put into them. They pushed out two updates in 2009, 0.4.11 and 0.4.12, but all they did for me was break basic compilation on multiple platforms. I still use 0.4.10 as the last version that seems to work without makefile surgery on both RedHat and Ubuntu. The last time I tried it, the read-only OLTP implementation worked fine, but the one that wrote instead was prone to deadlocks in PostgreSQL. -- Greg Smith 2ndQuadrant US Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.us -- 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] Policy decisions and cosmetic issues remaining for the git conversion
On 13/09/10 19:31, Tom Lane wrote: * If we do the above, should it be done in the existing CVS repository or just as part of the conversion to git? (I suspect it'd be a lot easier in git.) Similarly, ought we to fix the now-known tagging inconsistencies in the CVS repository, or just leave it for the conversion to deal with? Let's leave the CVS repository as it is. I don't want to destroy the evidence. * There are a number of partial tags (tags applied to just a subset of files) in the CVS repository: "MANUAL_1_0" and "SUPPORT" seem to have been applied to only documentation-related files, and "creation" and "Release-1-6-0" were applied only to src/interfaces/perl5/. I find the latter two particularly misleading since they have nothing to do with either creation of the whole project or a "release 1.6" of the whole project. These partial tags don't translate very well to git, either. I'm inclined to propose dropping all four. What was the purpose of these tags anyway? They don't seem useful, +1 for dropping all four. -- 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] Policy decisions and cosmetic issues remaining for the git conversion
On Mon, 2010-09-13 at 12:31 -0400, Tom Lane wrote: > This is an attempt to sum up the open issues remaining before we can > make another try at converting our source code to git. > > * As I noted previously, up till about 2003 we were quite haphazard about > applying CVS tags to identify the points where releases were made. Should > we try to clean that up? I think there is a stronger case for moving the > three existing misleading tags than for creating new tags matching the > releases that have none. Maybe nobody will ever care about any of them, > but if we are trying to create a good historical record it might be > appropriate to do it now while we have the information in mind. It is obnoxious but I think it is a good idea. > > * If we do the above, should it be done in the existing CVS repository > or just as part of the conversion to git? (I suspect it'd be a lot easier > in git.) Similarly, ought we to fix the now-known tagging inconsistencies > in the CVS repository, or just leave it for the conversion to deal with? IMO eventually the CVS repo will get deleted. We should do it where it is going to have he longest life, which would be git (or the conversion as it was) at this point (for at least another 15 years). Sincerely, Joshua D. Drake -- PostgreSQL.org Major Contributor Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579 Consulting, Training, Support, Custom Development, Engineering http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt -- 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] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
On 13 September 2010 17:27, Stefan Kaltenbrunner wrote: > On 09/13/2010 06:05 PM, Greg Smith wrote: >> >> Domas Mituzas wrote: >>> >>> I've been playing around today a lot with sysbench, and observed that >>> 2.6.32 kernel supplied by Ubuntu is having perf regression with PG >>> (which does not affect MySQL), compared to 2.6.28 builds I have. >>> What I observed can be seen in a paste at >>> http://p.defau.lt/?8_GQV82Pz3_SDZbNOdP93Q (db12 is 2.6.28, db20 is >>> 2.6.32 - 2.6.32-24-server). >>> Machines are two socket quad-opterons 2356s. >>> oprofile output can be seen at >>> http://p.defau.lt/?OIR1vDFK4cze_fmBTQbV9w - system has >20% of idle >>> cpu, which is somewhere in the top symbol :) >> >> Are you using the same filesystem setup on both setups? And regardless, >> what is that filesystem? We know that between 2.6.28 and 2.6.32 the >> kernel improved how it handles fsync requests in a good way from a >> reliability perspective (to fix bugs that could cause data loss before), >> particularly on ext4, so it's possible the regression you're seeing is >> just the expense of handling things properly. >> >> If you already have sysbench on there, I'd suggest comparing the two >> systems by seeing how fast each can execute fsync requests: >> >> sysbench --test=fileio --file-fsync-freq=1 --file-num=1 >> --file-total-size=16384 --file-test-mode=rndwr run | grep "Requests/sec" >> >> To help distinguish whether this regression might be coming from the >> already known changes in that area, or if it's instead from something >> that's impacting CPU efficiency. >> >> Also, it's easy to see a performance change of this size just from the >> database files being on a different part of the disk if you didn't >> control for that. Disks are almost twice as fast at their beginning than >> their end nowadays. > > well the main point here is that domas is doing a pure read-only test on a > rather small workload so it should entirely fit in memory... > From some very quick testing here as well it rathers seems that for some > reason the CPU scheduler is not actually scheduling us all the available CPU > on 2.6.32 or we are having some sort of locking issue that is more exposed > on this kernel. I thought sysbench was designed for MySQL benchmarks. How new is the PostgreSQL driver? Is it stable yet? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Policy decisions and cosmetic issues remaining for the git conversion
This is an attempt to sum up the open issues remaining before we can make another try at converting our source code to git. * As I noted previously, up till about 2003 we were quite haphazard about applying CVS tags to identify the points where releases were made. Should we try to clean that up? I think there is a stronger case for moving the three existing misleading tags than for creating new tags matching the releases that have none. Maybe nobody will ever care about any of them, but if we are trying to create a good historical record it might be appropriate to do it now while we have the information in mind. * If we do the above, should it be done in the existing CVS repository or just as part of the conversion to git? (I suspect it'd be a lot easier in git.) Similarly, ought we to fix the now-known tagging inconsistencies in the CVS repository, or just leave it for the conversion to deal with? * There are a number of partial tags (tags applied to just a subset of files) in the CVS repository: "MANUAL_1_0" and "SUPPORT" seem to have been applied to only documentation-related files, and "creation" and "Release-1-6-0" were applied only to src/interfaces/perl5/. I find the latter two particularly misleading since they have nothing to do with either creation of the whole project or a "release 1.6" of the whole project. These partial tags don't translate very well to git, either. I'm inclined to propose dropping all four. * The REL8_0_0 branch needs to be downgraded to a tag, as previously discussed. * There are several things we should do to make the manufactured commits less ugly, assuming we can't get rid of them entirely: ** Blame them on a nonexistent committer, probably "cvs2git" itself. ** If we do that, I'm inclined to also blame the inserted dead .0 revisions on cvs2git. Right now I copied-and-pasted the committer info from the mainline commit they inherit from, which is not very relevant. ** Make the manufactured-commit log messages say cvs2git not cvs2svn. ** Perhaps reword the log messages for the inserted dead .0 revisions? I didn't spend any time thinking about what they should say. * A more aggressive answer would be to collapse the manufactured commits for back-branch additions out of the history entirely, as was discussed briefly earlier. I'm not sure this is worth the trouble/risk. * There are a couple of manufactured commits that we could just delete, I think: the ones to create the Release_2_0 and Release_2_0_0 tags. The tags should be reapplied to the chronologically preceding mainline commits instead. This is just cosmetic but we may as well do it. I still think there's a cvs2git bug underlying those. Comments? 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] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
On 09/13/2010 06:05 PM, Greg Smith wrote: Domas Mituzas wrote: I've been playing around today a lot with sysbench, and observed that 2.6.32 kernel supplied by Ubuntu is having perf regression with PG (which does not affect MySQL), compared to 2.6.28 builds I have. What I observed can be seen in a paste at http://p.defau.lt/?8_GQV82Pz3_SDZbNOdP93Q (db12 is 2.6.28, db20 is 2.6.32 - 2.6.32-24-server). Machines are two socket quad-opterons 2356s. oprofile output can be seen at http://p.defau.lt/?OIR1vDFK4cze_fmBTQbV9w - system has >20% of idle cpu, which is somewhere in the top symbol :) Are you using the same filesystem setup on both setups? And regardless, what is that filesystem? We know that between 2.6.28 and 2.6.32 the kernel improved how it handles fsync requests in a good way from a reliability perspective (to fix bugs that could cause data loss before), particularly on ext4, so it's possible the regression you're seeing is just the expense of handling things properly. If you already have sysbench on there, I'd suggest comparing the two systems by seeing how fast each can execute fsync requests: sysbench --test=fileio --file-fsync-freq=1 --file-num=1 --file-total-size=16384 --file-test-mode=rndwr run | grep "Requests/sec" To help distinguish whether this regression might be coming from the already known changes in that area, or if it's instead from something that's impacting CPU efficiency. Also, it's easy to see a performance change of this size just from the database files being on a different part of the disk if you didn't control for that. Disks are almost twice as fast at their beginning than their end nowadays. well the main point here is that domas is doing a pure read-only test on a rather small workload so it should entirely fit in memory... From some very quick testing here as well it rathers seems that for some reason the CPU scheduler is not actually scheduling us all the available CPU on 2.6.32 or we are having some sort of locking issue that is more exposed on this kernel. Stefan -- 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] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
Domas Mituzas wrote: I've been playing around today a lot with sysbench, and observed that 2.6.32 kernel supplied by Ubuntu is having perf regression with PG (which does not affect MySQL), compared to 2.6.28 builds I have. What I observed can be seen in a paste at http://p.defau.lt/?8_GQV82Pz3_SDZbNOdP93Q (db12 is 2.6.28, db20 is 2.6.32 - 2.6.32-24-server). Machines are two socket quad-opterons 2356s. oprofile output can be seen at http://p.defau.lt/?OIR1vDFK4cze_fmBTQbV9w - system has >20% of idle cpu, which is somewhere in the top symbol :) Are you using the same filesystem setup on both setups? And regardless, what is that filesystem? We know that between 2.6.28 and 2.6.32 the kernel improved how it handles fsync requests in a good way from a reliability perspective (to fix bugs that could cause data loss before), particularly on ext4, so it's possible the regression you're seeing is just the expense of handling things properly. If you already have sysbench on there, I'd suggest comparing the two systems by seeing how fast each can execute fsync requests: sysbench --test=fileio --file-fsync-freq=1 --file-num=1 --file-total-size=16384 --file-test-mode=rndwr run | grep "Requests/sec" To help distinguish whether this regression might be coming from the already known changes in that area, or if it's instead from something that's impacting CPU efficiency. Also, it's easy to see a performance change of this size just from the database files being on a different part of the disk if you didn't control for that. Disks are almost twice as fast at their beginning than their end nowadays. -- Greg Smith 2ndQuadrant US Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com www.2ndQuadrant.us -- 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] cvs2git reports a "sprout" from a nonexistent commit?
Michael Haggerty writes: > On 09/13/2010 03:14 AM, Tom Lane wrote: >> Now as far as I can tell, the branch was made immediately before those >> test commits you can see Marc making in each branch. In particular, >> it was definitely made *after* Bryan deleted the src/bin/monitor files, >> because neither of them have REL2_0 or REL2_0B tags. So why did cvs2git >> choose to sprout the branch from the commit before that, and have to >> duplicate the deletion of the files? > This is a known bug 139 [1] in cvs2git's symbol parent choosing code > (previously mentioned as part of bug 55 [2]). Patches are welcome :-) Thanks for the response. Since this bug causes only one very minor infelicity in our conversion, probably nobody here is going to be motivated to fix it. OTOH, I'm still annoyed by the behavior for addition of files to back branches ... 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] Report: removing the inconsistencies in our CVS->git conversion
Robert Haas writes: > On Sun, Sep 12, 2010 at 11:03 PM, Tom Lane wrote: >> Having completed that comparison, I then moved on to trying to get rid of >> the discrepancies in the git conversion; particularly, trying to get rid >> of the "manufactured commits". I didn't have much success in that for the >> cases where the manufactured commit was caused by a back-branch file >> addition. [...] We still have "manufactured" commits either >> way, but they are just cosmetic so I guess we should live with them. > I'm not really following what the history looks like here. What are > the contents (git show) of the manufactured commit? A typical example is commit 4d2ac8075a93c685dbbe920f4bac23288dd7cf11 Author: PostgreSQL Daemon Date: Tue Nov 22 18:17:36 2005 + This commit was manufactured by cvs2svn to create branch 'REL7_4_STABLE'. Cherrypick from master 2005-11-22 18:17:34 UTC Bruce Momjian 'Re-run pgindent, fixing a problem where comment lines after a blank': src/port/unsetenv.c diff --git a/src/port/unsetenv.c b/src/port/unsetenv.c new file mode 100644 index 000..bdfb3f6 --- /dev/null +++ b/src/port/unsetenv.c @@ -0,0 +1,56 @@ + [ entire contents of unsetenv.c here ] In the cases where I inserted a dead .0 revision, this is followed by something like commit a1bdd263ca8ff657365a97a560f6371f39295efc Author: Bruce Momjian Date: Tue Nov 22 18:17:37 2005 + Mark branch as deleted. diff --git a/src/port/unsetenv.c b/src/port/unsetenv.c deleted file mode 100644 index bdfb3f6..000 --- a/src/port/unsetenv.c +++ /dev/null @@ -1,56 +0,0 @@ - [ entire contents of unsetenv.c here too ] I'm a bit disappointed by the fact that we get either of these. I had gathered from Max's comments that the dead-revision-at-the-base-of-the- branch trick is considered standard in newer CVS versions, and so I'd hoped that cvs2git would understand the construct and not generate either of these commits. Possibly the hacked-up revisions I inserted are enough different from the regular kind to confuse it. >> I also found numerous places where we'd been sloppy about placing tags. > I wonder if we should consider fixing some or all of these things on > the master CVS repository. I wouldn't be too eager to inject those > fake .0 commits for fear of breakage, but moving tags to where they > ought to have been all along seems like it might be a good thing to do > independent of git. Yeah, that's something I was wondering too. Applying these fixes to the master repository would also reduce the number of things we have to remember to do during the final conversion. OTOH, there's that risk of breaking something. 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] pg_ctl emits strange warning message
On 13/09/10 18:19, Tom Lane wrote: Heikki Linnakangas writes: Yet another idea: Check in pg_ctl if recovery.conf is also present. If it is, assume we're in recovery and don't print that warning. That would be dead simple. +1. This sounds like a much more appropriate approach. Hmm, looking at this more closely, I'm a bit confused. We already rename away backup_label at the beginning of recovery. Under what circumstances do we have a problem? This starts to seem like a complete non-issue to me. -- 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] pg_ctl emits strange warning message
Heikki Linnakangas writes: > Yet another idea: Check in pg_ctl if recovery.conf is also present. If > it is, assume we're in recovery and don't print that warning. That would > be dead simple. +1. This sounds like a much more appropriate approach. 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] pg_ctl emits strange warning message
On 13/09/10 16:01, Heikki Linnakangas wrote: On 13/09/10 13:08, Fujii Masao wrote: Hi, http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php (2) pg_ctl -ms stop emits the following warning whenever there is the backup_label file in $PGDATA. WARNING: online backup mode is active Shutdown will not complete until pg_stop_backup() is called. This warning doesn't fit in with the shutdown during recovery case. Since smart shutdown might be requested by other than pg_ctl, the warning should be emitted in server side rather than client, I think. How about moving the warning to the server side? Hmm, I'm not sure whether that's a good idea or not. Perhaps we should discuss for 9.1? Okay, this is not a critical problem. Let's revisit this issue. The problem here is that pg_ctl might emit the following warning messages when it shuts down the standby server. This is very strange thing since online backup mode is never active in the standby. WARNING: online backup mode is active Shutdown will not complete until pg_stop_backup() is called. Another problem is that the above useful messages are not emitted when shutdown is requested by other than pg_ctl. The attached patch moves the warning from pg_ctl to the server side and makes postmaster emit it only when online backup mode is really active. This can urge users to call pg_stop_backup to complete smart shutdown whenever it's requested during online backup. I added the patch into the next CF. There's this comment by Robert Haas: http://archives.postgresql.org/pgsql-hackers/2010-05/msg01433.php Like Robert, I'm also a bit worried that this might make the message less prominent. When you run "pg_ctl stop" manually, it's good to get the message directly in the terminal. Another line of attack is to remove the backup_label file earlier during recovery. We could remove it as soon as we update pg_control file with the same information, ie. the backup start location. And by remove I mean rename to backup_label.old, to avoid destroying forensic information ;-). Yet another idea: Check in pg_ctl if recovery.conf is also present. If it is, assume we're in recovery and don't print that warning. That would be dead simple. I would even consider backpatching it to 9.0, this can be regarded as a bug, albeit a minor one. -- 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] Report: removing the inconsistencies in our CVS->git conversion
Robert Haas writes: > Regrettably, all of your attachments came through as part of the > actual email, both in my GMail and in the archives. I hate > technology. Sorry about that. Here's another try with the stuff in a tarball. This time, I also remembered to include cvs2git.options; although I think it's the same as Max's original except for -r'cvsroot/pgsql', +r'/cvsroot/pgsql', I'll address the other points in a bit. regards, tom lane bind9DKoETd5P.bin Description: conv.tar.gz -- 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] knngist - 0.8
http://www.sigaev.ru/misc/builtin_knngist_core-0.9.gz http://www.sigaev.ru/misc/builtin_knngist_itself-0.8.2.gz http://www.sigaev.ru/misc/builtin_knngist_proc-0.8.gz http://www.sigaev.ru/misc/builtin_knngist_contrib_pg_trgm-0.8.gz http://www.sigaev.ru/misc/builtin_knngist_contrib_btree_gist-0.8.1.gz + if (opform->oprresult == BOOLOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), +errmsg("index ordering operators must not return boolean"))); New version allows to use boolean distance, i.e. the same operator could be used both in WHERE and ORDER BY clauses. Now operator could present in operator family twice, but with different StrategyNumber: as ordinary search operator (in WHERE clause) and as an order operator (ORDER BY). So, list of changes: 1) "pg_amop_opr_fam_index" UNIQUE, btree (amopopr, amopfamily) => "pg_amop_opr_fam_index" UNIQUE, btree (amopopr, amopfamily, amoporder) 2) op_in_opfamily, get_op_opfamily_strategy, get_op_opfamily_properties accept one more argument, bool orderop, to point which kind of operator we need to find 3) bool OpExpr.orderop. This field is needed to correctly set SK_ORDER flags for index scan (ExecIndexBuildScanKeys function). SK_ORDER flag points type of tree traversal algorithm to GiST core. Introducing two fields, per Tom's suggestion, doesn't resolve following issues: - we still need to distinguish WHERE and ORDER BY operator in ExecIndexBuildScanKey, as it done in new version of patch - When we execute AMOPOPID cache request we still need to check pg_amop.amorder or introduce two new indexes (amopopr, amopfamily, amopcanwhere) and (amopopr, amopfamily, amopcanorder) and the same changes in lsyscache's functions - If one operation could be used for both usage type then it will be good to distinguish them in consistent method. New version requires to use different strategy number for each usage type. -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ -- 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] top-level DML under CTEs
On Mon, Sep 13, 2010 at 9:15 AM, Hitoshi Harada wrote: > The patch attached is based on the one rejected at the last CF for 9.0 > last year. > > http://archives.postgresql.org/message-id/16303.1266023...@sss.pgh.pa.us > > This patch implements the feature that allows top-level DMLs under CTE > WITH clause. For example: > > WITH t AS (SELECT * FROM x) > UPDATE y SET val = t.val FROM t > WHERE y.key = t.key; > > This feature is part of writeable CTEs proposed by David Fetter originally. Thanks for pursuing this. I think this will be a useful feature if we can get it committed, and plus David Fetter will be very, very happy. :-) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] top-level DML under CTEs
The patch attached is based on the one rejected at the last CF for 9.0 last year. http://archives.postgresql.org/message-id/16303.1266023...@sss.pgh.pa.us This patch implements the feature that allows top-level DMLs under CTE WITH clause. For example: WITH t AS (SELECT * FROM x) UPDATE y SET val = t.val FROM t WHERE y.key = t.key; This feature is part of writeable CTEs proposed by David Fetter originally. There were two issues at the CF. 1. WITH clause atop INSERT Although the previous discussion got the consensus that we forbid WITH atop INSERT, it seems to me that it can be allowed. I managed to do it by treating the top WITH clause (of INSERT) as if the one of SELECT (or VALUES). It is possible to disallow the CTE over INSERT statement, but the lack for INSERT, though there are for UPDATE and DELETE, sounds inconsistent enough. 2. OLD/NEW in rules Following the subsequent discussion after the post linked above, I add code to throw an appropriate error when OLD/NEW is used in WITH clauses. It is true that OLD/NEW references look sane to general users, but actually (at least in our implementation) they are located in the top-level query's Range Table List. Consequently, they are invisible inside the WITH clause. To allow them, we should rewrite the rule systems overall. Thus, we forbid them in WITH though we should throw an error indicating appropriate message. I'll add the entry to CF app later. Any feedback is welcome. Regards, -- Hitoshi Harada toplevel-dml-cte.20100913.patch 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
Re: [HACKERS] pg_ctl emits strange warning message
On 13/09/10 13:08, Fujii Masao wrote: Hi, http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php (2) pg_ctl -ms stop emits the following warning whenever there is the backup_label file in $PGDATA. WARNING: online backup mode is active Shutdown will not complete until pg_stop_backup() is called. This warning doesn't fit in with the shutdown during recovery case. Since smart shutdown might be requested by other than pg_ctl, the warning should be emitted in server side rather than client, I think. How about moving the warning to the server side? Hmm, I'm not sure whether that's a good idea or not. Perhaps we should discuss for 9.1? Okay, this is not a critical problem. Let's revisit this issue. The problem here is that pg_ctl might emit the following warning messages when it shuts down the standby server. This is very strange thing since online backup mode is never active in the standby. WARNING: online backup mode is active Shutdown will not complete until pg_stop_backup() is called. Another problem is that the above useful messages are not emitted when shutdown is requested by other than pg_ctl. The attached patch moves the warning from pg_ctl to the server side and makes postmaster emit it only when online backup mode is really active. This can urge users to call pg_stop_backup to complete smart shutdown whenever it's requested during online backup. I added the patch into the next CF. There's this comment by Robert Haas: http://archives.postgresql.org/pgsql-hackers/2010-05/msg01433.php Like Robert, I'm also a bit worried that this might make the message less prominent. When you run "pg_ctl stop" manually, it's good to get the message directly in the terminal. Another line of attack is to remove the backup_label file earlier during recovery. We could remove it as soon as we update pg_control file with the same information, ie. the backup start location. And by remove I mean rename to backup_label.old, to avoid destroying forensic information ;-). -- 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] security label support, revised
On Mon, Sep 13, 2010 at 8:38 AM, KaiGai Kohei wrote: > Yes, if and when MAC-X and MAC-Y are installed, it is significant event > for MAC-X to change X's label, so MAC-X may need to check special > permissions. But it is a common event for MAC-Y and DAC, so they checks > an appropriate permission to change one of the properties. Hoever, it > does not mean we should not give any chance MAC-Y and DAC to check > something. > > I'll revise my patch within a couple of days. I have a feeling we are talking past each other. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] security label support, revised
(2010/09/13 20:46), Robert Haas wrote: 2010/9/13 KaiGai Kohei: Robert, although you suggested that only one ESP module shall be invoked on relabeling an object before, and I agreed this design at that time, but I noticed that we cannot implement the following behavior correctly. SELinux defines these permissions corresponding to table relabeling. - db_table:{setattr} It is necessary to change *any* properties of the table. Security label is one of properties of the table, so, needs to be checked on relabeling, not only ALTER TABLE and so on. - db_table:{relabelfrom relabelto} It is neccesary to change label of the table. User must have {relabelfrom} permission on the older security label, and {relabelto} permission on the new security label. If and when multiple ESP modules are installed, we need to consider the way to handle SECURITY LABEL statement for other modules. When user tries to relabel security label of a table managed by something except for selinux, it is not relevant to {relabelfrom relabelto} permission in selinux, but we want to check {setattr} permission on the operation, because it is a property of the table, although not a security label in selinux. In the current patch, the core PG (ExecSecurityLabel()) invokes only one hook that matches with the given "FOR" option. However, I reconsidered this hook should be simply invoked like other hooks. Then, the ESP module decides whether ignores or handles the invocation, and also decides to call secondary hook when multiple modules are loaded. If so, selinux module can check {setattr} and also calls secondary modules. In the previous discussion, I missed the possibility of the case when we want to check relabeling a security label managed by other ESP. But it might be also necessary to call all the modules which want to get control on SECURITY LABEL statement, apart from whether it manages the supplied security label, or not. Maybe. The whole point of MAC is that the security policy itself is capable of enforcing all of the necessary protections. It shouldn't be necessary for it to control what DAC or other MAC providers do, should it? Yes, what we should do here is that DAC and any MACs make their own decision individually, then PG eventually prevents user's request if one of them denied at least. At any rate, they'll probably treat it quite a bit differently than a change of their own label. I think it might be cleaner to fold that in under some of the DDL permissions checking and refactoring which we know still needs to be done, rather than cramming it in here. Yes, if and when MAC-X and MAC-Y are installed, it is significant event for MAC-X to change X's label, so MAC-X may need to check special permissions. But it is a common event for MAC-Y and DAC, so they checks an appropriate permission to change one of the properties. Hoever, it does not mean we should not give any chance MAC-Y and DAC to check something. I'll revise my patch within a couple of days. Thanks, Note that presumably COMMENT ON would need similar treatment, and there may be other things. -- KaiGai Kohei -- 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] Report: removing the inconsistencies in our CVS->git conversion
On Sun, Sep 12, 2010 at 11:03 PM, Tom Lane wrote: > I've spent much of the weekend examining the discrepancies between our CVS > repository and the tarballs available from our FTP archives, and after > that trying to remove infelicities in the cvs2git output. There are a > couple of remaining oddities that I would classify as probable cvs2git > bugs, but an awful lot of it is inconsistencies in the CVS repository > itself, some of which I can explain and some that I can't. Read on for > many boring details. First of all, WOW, and thank you very much for putting in the time to make this happen. > With those changes, I am able to match all the available archival tarballs > to various places in the CVS history. The exact spots where they match > are detailed in the attached "matches" file. The file also shows the Regrettably, all of your attachments came through as part of the actual email, both in my GMail and in the archives. I hate technology. > Having completed that comparison, I then moved on to trying to get rid of > the discrepancies in the git conversion; particularly, trying to get rid > of the "manufactured commits". I didn't have much success in that for the > cases where the manufactured commit was caused by a back-branch file > addition. [...] We still have "manufactured" commits either > way, but they are just cosmetic so I guess we should live with them. I'm not really following what the history looks like here. What are the contents (git show) of the manufactured commit? > I also found numerous places where we'd been sloppy about placing tags. > That explains some of the weird things cvs2git did. In particular: > > * We had the already-known problem that gram.c and some other derived > files had commits made after they should have been dead. > > * Bruce had transiently added those files on the WIN32_DEV branch as > well, to general disapproval, and this seemed to also give cvs2git > indigestion. The attached proposed fixup script deals with this by > deleting those revisions altogether. This is a loss of history, but > not one that I care about. > > * The HISTORY and INSTALL files have REL7_3_10 tags and should not. > As mentioned earlier, I think this is because they were deleted after the > original placement of that tag, and weren't correctly fixed when the > tag was moved up to branch end a few days later. > > * The regression tests files recently added to contrib/xml2 have REL8_0_23 > tags. I have no idea how that happened, because they certainly didn't > exist when 8.0.23 was released. > > * There are a bunch of files that should have REL7_3_5 tags and lack them. > They are in just a few subdirectories, so probably what happened was that > the "cvs tag" operation was issued in an incomplete checkout tree. > > * Similarly, gram.c should have a release-6-3 tag and lacks it. > > * There are a bunch of files that have REL7_1 tags when what they should > have are REL7_1_BETA tags. These appear to be exactly the files that were > deleted between the initial placement of the REL7_1 tag and Marc's later > ex-post-facto renaming of the tag to REL7_1_BETA. I'm guessing another > case of "cvs tag" missing files that weren't in the checkout. > > * There are a number of files that lack the REL2_0 tag and REL2_0B branch, > though they should have it according to file dates. These appear to be > exactly the files that were in the separate documentation repository at > the time, so that probably tells us the mechanism for missing them. I wonder if we should consider fixing some or all of these things on the master CVS repository. I wouldn't be too eager to inject those fake .0 commits for fear of breakage, but moving tags to where they ought to have been all along seems like it might be a good thing to do independent of git. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Reducing walreceiver latency with a latch
On 13/09/10 14:54, Heikki Linnakangas wrote: BTW, I noticed that I missed incrementing the latch count in win32_latch.c, and the owning/disowning the latch was done correctly, you get an error if you restart the master and reconnect. I'll post an updated patch shortly. Here's an updated patch with those bugs fixed. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ddf7d79..40e1718 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -46,6 +46,7 @@ #include "storage/bufmgr.h" #include "storage/fd.h" #include "storage/ipc.h" +#include "storage/latch.h" #include "storage/pmsignal.h" #include "storage/procarray.h" #include "storage/smgr.h" @@ -9139,6 +9140,13 @@ startupproc_quickdie(SIGNAL_ARGS) } +/* SIGUSR1: let latch facility handle the signal */ +static void +StartupProcSigUsr1Handler(SIGNAL_ARGS) +{ + latch_sigusr1_handler(); +} + /* SIGHUP: set flag to re-read config file at next convenient time */ static void StartupProcSigHupHandler(SIGNAL_ARGS) @@ -9213,7 +9221,7 @@ StartupProcessMain(void) else pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, SIG_IGN); + pqsignal(SIGUSR1, StartupProcSigUsr1Handler); pqsignal(SIGUSR2, SIG_IGN); /* @@ -9397,16 +9405,13 @@ retry: } /* - * Data not here yet, so check for trigger then sleep. + * Data not here yet, so check for trigger then sleep for + * five seconds like in the WAL file polling case below. */ if (CheckForStandbyTrigger()) goto triggered; - /* - * When streaming is active, we want to react quickly when - * the next WAL record arrives, so sleep only a bit. - */ - pg_usleep(10L); /* 100ms */ + WaitForWalArrival(500L); } else { diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c index da06202..e39bf1c 100644 --- a/src/backend/port/win32_latch.c +++ b/src/backend/port/win32_latch.c @@ -230,6 +230,8 @@ NumSharedLatches(void) /* Each walsender needs one latch */ numLatches += max_wal_senders; + /* One latch for startup process - walreceiver communication */ + numLatches += 1; return numLatches; } diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index b868707..996c54f 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -182,6 +182,12 @@ WalReceiverMain(void) Assert(walrcv->pid == 0); switch (walrcv->walRcvState) { + case WALRCV_NOT_INITIALIZED: + /* can't happen */ + SpinLockRelease(&walrcv->mutex); + elog(ERROR, "walreceiver state not initialized"); + break; + case WALRCV_STOPPING: /* If we've already been requested to stop, don't start up. */ walrcv->walRcvState = WALRCV_STOPPED; @@ -529,6 +535,9 @@ XLogWalRcvFlush(void) walrcv->receivedUpto = LogstreamResult.Flush; SpinLockRelease(&walrcv->mutex); + /* Signal the startup process that new WAL has arrived */ + SetLatch(&walrcv->receivedLatch); + /* Report XLOG streaming progress in PS display */ if (update_process_title) { diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c index b206885..571fc02 100644 --- a/src/backend/replication/walreceiverfuncs.c +++ b/src/backend/replication/walreceiverfuncs.c @@ -62,8 +62,9 @@ WalRcvShmemInit(void) { /* First time through, so initialize */ MemSet(WalRcv, 0, WalRcvShmemSize()); - WalRcv->walRcvState = WALRCV_STOPPED; + WalRcv->walRcvState = WALRCV_NOT_INITIALIZED; SpinLockInit(&WalRcv->mutex); + InitSharedLatch(&WalRcv->receivedLatch); } } @@ -104,7 +105,7 @@ WalRcvInProgress(void) } } - if (state != WALRCV_STOPPED) + if (state != WALRCV_STOPPED && state != WALRCV_NOT_INITIALIZED) return true; else return false; @@ -128,6 +129,7 @@ ShutdownWalRcv(void) SpinLockAcquire(&walrcv->mutex); switch (walrcv->walRcvState) { + case WALRCV_NOT_INITIALIZED: case WALRCV_STOPPED: break; case WALRCV_STARTING: @@ -177,6 +179,7 @@ RequestXLogStreaming(XLogRecPtr recptr, const char *conninfo) /* use volatile pointer to prevent code rearrangement */ volatile WalRcvData *walrcv = WalRcv; pg_time_t now = (pg_time_t) time(NULL); + bool firsttime; /* * We always start at the beginning of the segment. That prevents a broken @@ -187,8 +190,20 @@ RequestXLogStreaming(XLogRecPtr recptr, const char *conninfo) if (recptr.xrecoff % XLogSegSize != 0) recptr.xrecoff -= recptr.xrecoff % XLogSegSize; + /* + * Update shared memory status with information needed by walreceiver + */ SpinLockAcquire(&walrcv->mutex); + /* Check if this is the first time we start walreceiver */ + if (walrcv->walRcvState == WALRCV_NOT_INITIALIZED) + { + firsttime = true; + walrcv->walRcvState = WALRCV_STOPPED; + } + else + firsttime = fal
Re: [HACKERS] [RRR] CommitFest 2010-07 final report
On Mon, Sep 13, 2010 at 7:44 AM, Thom Brown wrote: > So did the materialized views patch not get submitted? I think someone else will need to pick it up and do a bunch more work with it before it can be considered a serious candidate for commit. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] update on global temporary and unlogged tables
On Mon, Sep 13, 2010 at 5:24 AM, Heikki Linnakangas wrote: > The LSNs on all pages in an unlogged relation should be zero, and > XLogFlush() will do nothing. That's what we rely on at the moment for pages > that are not WAL-logged for some reason, I don't think you need any extra > flag for that. Ah, interesting. I wonder if I should add a cross-check for that and elog(LOG) if it isn't the case, or some such. >> So I went looking for bit-space in the buffer tag and quickly found >> some. ForkNumber is an enum which I suppose means a 32-bit integer, >> but we've only got three forks right now and it's hard to imagine more >> than a handful of additional ones, so what I'm tempted to do is change >> this from an enum to a 2-byte integer and replace the enum values with >> #defines. That frees up 2 bytes in the buffer tag which is more than >> plenty. > > I haven't been following the discussion so I don't understand why you need > the extra bits, but no objections to reducing the fork number field. Well, the idea is that unlogged table files are named differently than regular table files (I'm thinking, just insert a "u" at the beginning) so that we can truncate them at startup time without needing to look at any catalog entries. So the point is you could have data/base/16384/u124141421 block 173 and data/base/16384/124141421 block 173 in the buffer cache at the same time. The alternative is to try to make sure that you never create both of those files in the first place, but that requires an extra system call per GetNewRelFileNode() - and that could get worse in the future if for some reason we find it advantageous to have more than two "namespaces". (The already-committed RelFileNodeBackend patch already creates one "namespace" per backend for temporary tables plus one for permanent tables; but that doesn't run into this problem because temporary tables use backend-local buffers - i.e. the relevant BackendId can be inferred strictly from which set of buffers we're looking at.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] Reducing walreceiver latency with a latch
On 13/09/10 14:47, Thom Brown wrote: On 13 September 2010 12:40, Heikki Linnakangas wrote: Now that we have the wonderful latch facility, let's use it to reduce the delay between receiving a piece of WAL and applying in the standby. Currently, the startup process polls every 100ms to see if new WAL has arrived, which adds an average a 50 ms delay between a transaction commit in the master and it appearing as committed in a hot standby server. The latch patch eliminated a similar polling delay in walsender already, the attached patch does the same for walreceiver. After this patch, there is no unnecessary delays in the streaming replication code path. Note that this is all still asynchronous, just with reduced latency. This is pretty straightforward, but any comments? Is that supposed to be waiting 5000ms? Yes, it gets interrupted as soon as WAL arrives, that timeout is to poll for the standby trigger file to appear or SIGTERM. BTW, I noticed that I missed incrementing the latch count in win32_latch.c, and the owning/disowning the latch was done correctly, you get an error if you restart the master and reconnect. I'll post an updated patch shortly. -- 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] Reducing walreceiver latency with a latch
On 13 September 2010 12:47, Thom Brown wrote: > On 13 September 2010 12:40, Heikki Linnakangas > wrote: >> Now that we have the wonderful latch facility, let's use it to reduce the >> delay between receiving a piece of WAL and applying in the standby. >> Currently, the startup process polls every 100ms to see if new WAL has >> arrived, which adds an average a 50 ms delay between a transaction commit in >> the master and it appearing as committed in a hot standby server. The latch >> patch eliminated a similar polling delay in walsender already, the attached >> patch does the same for walreceiver. >> >> After this patch, there is no unnecessary delays in the streaming >> replication code path. Note that this is all still asynchronous, just with >> reduced latency. >> >> This is pretty straightforward, but any comments? > > Is that supposed to be waiting 5000ms? Ignore me, I can see that it's right. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- 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] Reducing walreceiver latency with a latch
On 13 September 2010 12:40, Heikki Linnakangas wrote: > Now that we have the wonderful latch facility, let's use it to reduce the > delay between receiving a piece of WAL and applying in the standby. > Currently, the startup process polls every 100ms to see if new WAL has > arrived, which adds an average a 50 ms delay between a transaction commit in > the master and it appearing as committed in a hot standby server. The latch > patch eliminated a similar polling delay in walsender already, the attached > patch does the same for walreceiver. > > After this patch, there is no unnecessary delays in the streaming > replication code path. Note that this is all still asynchronous, just with > reduced latency. > > This is pretty straightforward, but any comments? Is that supposed to be waiting 5000ms? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- 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] security label support, revised
2010/9/13 KaiGai Kohei : > Robert, although you suggested that only one ESP module shall be > invoked on relabeling an object before, and I agreed this design > at that time, but I noticed that we cannot implement the following > behavior correctly. > > SELinux defines these permissions corresponding to table relabeling. > - db_table:{setattr} > It is necessary to change *any* properties of the table. > Security label is one of properties of the table, so, needs to be > checked on relabeling, not only ALTER TABLE and so on. > - db_table:{relabelfrom relabelto} > It is neccesary to change label of the table. > User must have {relabelfrom} permission on the older security label, > and {relabelto} permission on the new security label. > > If and when multiple ESP modules are installed, we need to consider > the way to handle SECURITY LABEL statement for other modules. > When user tries to relabel security label of a table managed by > something except for selinux, it is not relevant to {relabelfrom > relabelto} permission in selinux, but we want to check {setattr} > permission on the operation, because it is a property of the table, > although not a security label in selinux. > > In the current patch, the core PG (ExecSecurityLabel()) invokes only > one hook that matches with the given "FOR " option. > However, I reconsidered this hook should be simply invoked like other > hooks. Then, the ESP module decides whether ignores or handles the > invocation, and also decides to call secondary hook when multiple > modules are loaded. If so, selinux module can check {setattr} and > also calls secondary modules. > > In the previous discussion, I missed the possibility of the case > when we want to check relabeling a security label managed by other > ESP. But it might be also necessary to call all the modules which > want to get control on SECURITY LABEL statement, apart from whether > it manages the supplied security label, or not. Maybe. The whole point of MAC is that the security policy itself is capable of enforcing all of the necessary protections. It shouldn't be necessary for it to control what DAC or other MAC providers do, should it? At any rate, they'll probably treat it quite a bit differently than a change of their own label. I think it might be cleaner to fold that in under some of the DDL permissions checking and refactoring which we know still needs to be done, rather than cramming it in here. Note that presumably COMMENT ON would need similar treatment, and there may be other things. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- 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] [RRR] CommitFest 2010-07 final report
On 18 August 2010 22:45, Kevin Grittner wrote: > At the close of the 2010-07 CommitFest, the numbers were: > > 72 patches were submitted > 3 patches were withdrawn (deleted) by their authors > 14 patches were moved to CommitFest 2010-09 > -- > 55 patches in CommitFest 2010-07 > -- > 3 committed to 9.0 > -- > 52 patches for 9.1 > -- > 1 rejected > 20 returned with feedback > 31 committed for 9.1 > > When we hit the end of the allotted time, I moved the last two > patches to the next CF, for want of a better idea for disposition. > One is "Ready for Committer" with an author who is a committer. The > other is my WiP patch for serializable transactions -- there's a lot > to review and the reviewer had unexpected demands on his time during > the CF; he said he'll continue work on that outside the CF. > > -Kevin > > > At the end of week four: > >> 72 patches were submitted >> 3 patches were withdrawn (deleted) by their authors >> 12 patches were moved to CommitFest 2010-09 >> -- >> 57 patches in CommitFest 2010-07 >> -- >> 3 committed to 9.0 >> -- >> 54 patches for 9.1 >> -- >> 1 rejected >> 18 returned with feedback >> 28 committed for 9.1 >> -- >> 47 disposed >> -- >> 7 pending >> 2 ready for committer >> -- >> 5 will still need reviewer attention >> 1 waiting on author to respond to review >> -- >> 4 patches need review now and have a reviewer assigned So did the materialized views patch not get submitted? -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Reducing walreceiver latency with a latch
Now that we have the wonderful latch facility, let's use it to reduce the delay between receiving a piece of WAL and applying in the standby. Currently, the startup process polls every 100ms to see if new WAL has arrived, which adds an average a 50 ms delay between a transaction commit in the master and it appearing as committed in a hot standby server. The latch patch eliminated a similar polling delay in walsender already, the attached patch does the same for walreceiver. After this patch, there is no unnecessary delays in the streaming replication code path. Note that this is all still asynchronous, just with reduced latency. This is pretty straightforward, but any comments? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index ddf7d79..40e1718 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -46,6 +46,7 @@ #include "storage/bufmgr.h" #include "storage/fd.h" #include "storage/ipc.h" +#include "storage/latch.h" #include "storage/pmsignal.h" #include "storage/procarray.h" #include "storage/smgr.h" @@ -9139,6 +9140,13 @@ startupproc_quickdie(SIGNAL_ARGS) } +/* SIGUSR1: let latch facility handle the signal */ +static void +StartupProcSigUsr1Handler(SIGNAL_ARGS) +{ + latch_sigusr1_handler(); +} + /* SIGHUP: set flag to re-read config file at next convenient time */ static void StartupProcSigHupHandler(SIGNAL_ARGS) @@ -9213,7 +9221,7 @@ StartupProcessMain(void) else pqsignal(SIGALRM, SIG_IGN); pqsignal(SIGPIPE, SIG_IGN); - pqsignal(SIGUSR1, SIG_IGN); + pqsignal(SIGUSR1, StartupProcSigUsr1Handler); pqsignal(SIGUSR2, SIG_IGN); /* @@ -9397,16 +9405,13 @@ retry: } /* - * Data not here yet, so check for trigger then sleep. + * Data not here yet, so check for trigger then sleep for + * five seconds like in the WAL file polling case below. */ if (CheckForStandbyTrigger()) goto triggered; - /* - * When streaming is active, we want to react quickly when - * the next WAL record arrives, so sleep only a bit. - */ - pg_usleep(10L); /* 100ms */ + WaitForWalArrival(500L); } else { diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index b868707..e12f1f5 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -529,6 +529,9 @@ XLogWalRcvFlush(void) walrcv->receivedUpto = LogstreamResult.Flush; SpinLockRelease(&walrcv->mutex); + /* Signal the startup process that new WAL has arrived */ + SetLatch(&walrcv->receivedLatch); + /* Report XLOG streaming progress in PS display */ if (update_process_title) { diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c index b206885..8182160 100644 --- a/src/backend/replication/walreceiverfuncs.c +++ b/src/backend/replication/walreceiverfuncs.c @@ -64,6 +64,7 @@ WalRcvShmemInit(void) MemSet(WalRcv, 0, WalRcvShmemSize()); WalRcv->walRcvState = WALRCV_STOPPED; SpinLockInit(&WalRcv->mutex); + InitSharedLatch(&WalRcv->receivedLatch); } } @@ -163,6 +164,9 @@ ShutdownWalRcv(void) pg_usleep(10); /* 100ms */ } + + /* We don't need the latch anymore */ + DisownLatch(&walrcv->receivedLatch); } /* @@ -187,6 +191,9 @@ RequestXLogStreaming(XLogRecPtr recptr, const char *conninfo) if (recptr.xrecoff % XLogSegSize != 0) recptr.xrecoff -= recptr.xrecoff % XLogSegSize; + /* + * Update shared memory status with information needed by walreceiver + */ SpinLockAcquire(&walrcv->mutex); /* It better be stopped before we try to restart it */ @@ -204,6 +211,10 @@ RequestXLogStreaming(XLogRecPtr recptr, const char *conninfo) SpinLockRelease(&walrcv->mutex); + /* Take ownership of the latch so that we can wait on it */ + OwnLatch(&walrcv->receivedLatch); + + /* Request postmaster to start the walreceiver process */ SendPostmasterSignal(PMSIGNAL_START_WALRECEIVER); } @@ -229,3 +240,20 @@ GetWalRcvWriteRecPtr(XLogRecPtr *latestChunkStart) return recptr; } + +/* + * Wait for more WAL to arrive, or timeout (in microseconds) to be reached + */ +void +WaitForWalArrival(int timeout) +{ + /* Wait for more WAL to arrive */ + if (WaitLatch(&WalRcv->receivedLatch, timeout)) + { + /* + * Reset the latch so that next call to WaitForWalArrival will sleep + * again. + */ + ResetLatch(&WalRcv->receivedLatch); + } +} diff --git a/src/include/replication/walreceiver.h b/src/include/replication/walreceiver.h index 2ea881e..66a8229 100644 --- a/src/include/replication/walreceiver.h +++ b/src/include/replication/walreceiver.h @@ -13,6 +13,7 @@ #define _WALRECEIVER_H #include "access/xlogdefs.h" +#include "storage/latch.h" #include "storage/spin.h" #include "pgtime.h" @@ -72,6 +73,13 @@ typedef struct char conninfo[MAXCONNINFO]; s
[HACKERS] pg_ctl emits strange warning message
Hi, http://archives.postgresql.org/pgsql-hackers/2010-05/msg00921.php >>> (2) >>> pg_ctl -ms stop emits the following warning whenever there is the >>> backup_label file in $PGDATA. >>> >>> WARNING: online backup mode is active >>> Shutdown will not complete until pg_stop_backup() is called. >>> >>> This warning doesn't fit in with the shutdown during recovery case. >>> Since smart shutdown might be requested by other than pg_ctl, the >>> warning should be emitted in server side rather than client, I think. >>> How about moving the warning to the server side? >> >> Hmm, I'm not sure whether that's a good idea or not. Perhaps we >> should discuss for 9.1? > > Okay, this is not a critical problem. Let's revisit this issue. The problem here is that pg_ctl might emit the following warning messages when it shuts down the standby server. This is very strange thing since online backup mode is never active in the standby. WARNING: online backup mode is active Shutdown will not complete until pg_stop_backup() is called. Another problem is that the above useful messages are not emitted when shutdown is requested by other than pg_ctl. The attached patch moves the warning from pg_ctl to the server side and makes postmaster emit it only when online backup mode is really active. This can urge users to call pg_stop_backup to complete smart shutdown whenever it's requested during online backup. I added the patch into the next CF. Comments? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center warning_during_shutdown_v1.patch 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
Re: [HACKERS] Perf regression in 2.6.32 (Ubuntu 10.04 LTS)
On 12/09/10 23:31, Domas Mituzas wrote: I've been playing around today a lot with sysbench, and observed that 2.6.32 kernel supplied by Ubuntu is having perf regression with PG (which does not affect MySQL), compared to 2.6.28 builds I have. What I observed can be seen in a paste at http://p.defau.lt/?8_GQV82Pz3_SDZbNOdP93Q (db12 is 2.6.28, db20 is 2.6.32 - 2.6.32-24-server). Machines are two socket quad-opterons 2356s. oprofile output can be seen at http://p.defau.lt/?OIR1vDFK4cze_fmBTQbV9w - system has>20% of idle cpu, which is somewhere in the top symbol :) Can you run oprofile on the older kernel, so that we can compare and see where the time is spent? Looks like over 7% of the time is spent in s_lock, which suggests some change in behavior in context switching or something like that, but let's see what the old profile looks like. -- 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] update on global temporary and unlogged tables
On 13/09/10 05:49, Robert Haas wrote: On Mon, Sep 6, 2010 at 10:55 PM, Robert Haas wrote: 3. With respect to unlogged tables, the major obstacle seems to be figuring out a way for these to get automatically truncated at startup time. As with temporary table cleanup in general, the problem here is that you can't do the obvious thing of iterating through pg_class and truncating each unlogged table you find without greatly complicating the startup sequence. However, I think there's a fairly easy way around this problem: truncating a table basically means removing all segments and relation forks other than the first segment of the main fork, and truncating that one to zero length. So we could do it the same way we clean up temporary files - namely, based on the file name - if we made the filenames for unlogged tables distinguishable from those for regular and temporary tables. What I'm thinking about is reserving a backend ID of -2 for this purpose via some suitable constant definition, just as -1 (InvalidBackendId) represents a permanent table in this context. I tried this approach and got fairly far with it, but ran into a snag in the buffer manager. It's fairly obvious that the buffer manager has to know whether a particular buffer is from an unlogged relation or not; for example, FlushBuffer() should skip the XLOG flush for an unlogged buffer, and must pass the correct backend ID to smgropen(). So my first thought was just to define a bit BM_IS_UNLOGGED, with the obvious interpretation. The LSNs on all pages in an unlogged relation should be zero, and XLogFlush() will do nothing. That's what we rely on at the moment for pages that are not WAL-logged for some reason, I don't think you need any extra flag for that. So I went looking for bit-space in the buffer tag and quickly found some. ForkNumber is an enum which I suppose means a 32-bit integer, but we've only got three forks right now and it's hard to imagine more than a handful of additional ones, so what I'm tempted to do is change this from an enum to a 2-byte integer and replace the enum values with #defines. That frees up 2 bytes in the buffer tag which is more than plenty. I haven't been following the discussion so I don't understand why you need the extra bits, but no objections to reducing the fork number field. -- 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] Walsender doesn't process options passed in the startup packet
On 13/09/10 08:10, Fujii Masao wrote: Okay. I got rid of the access to pg_db_role_setting from the patch. I attached the updated version, which makes walsender process the options passed in the startup packet, apply PostAuthDelay and initialize client encoding. Thanks, committed. I moved the check for "MyProcPort == NULL" case to the callers of process_startup_packet(), it seems more logical to me. -- 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] cvs2git reports a "sprout" from a nonexistent commit?
On 09/13/2010 03:14 AM, Tom Lane wrote: [...] Now as far as I can tell, the branch was made immediately before those test commits you can see Marc making in each branch. In particular, it was definitely made *after* Bryan deleted the src/bin/monitor files, because neither of them have REL2_0 or REL2_0B tags. So why did cvs2git choose to sprout the branch from the commit before that, and have to duplicate the deletion of the files? This is a known bug 139 [1] in cvs2git's symbol parent choosing code (previously mentioned as part of bug 55 [2]). Patches are welcome :-) Michael [1] http://cvs2svn.tigris.org/issues/show_bug.cgi?id=139 [2] http://cvs2svn.tigris.org/issues/show_bug.cgi?id=55 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers