Re: [HACKERS] Report: removing the inconsistencies in our CVS->git conversion

2010-09-13 Thread Bruce Momjian
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

2010-09-13 Thread Robert Haas
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)

2010-09-13 Thread Andrew Dunstan



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

2010-09-13 Thread Fujii Masao
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)

2010-09-13 Thread Itagaki Takahiro
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

2010-09-13 Thread Rob Wultsch
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

2010-09-13 Thread Fujii Masao
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-09-13 Thread Hitoshi Harada
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

2010-09-13 Thread Fujii Masao
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Bruce Momjian
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

2010-09-13 Thread Kevin Grittner
>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?

2010-09-13 Thread Bruce Momjian
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

2010-09-13 Thread Magnus Hagander
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread 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
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Magnus Hagander
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

2010-09-13 Thread Magnus Hagander
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Robert Haas
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!)

2010-09-13 Thread Heikki Linnakangas

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!)

2010-09-13 Thread Jeff Davis
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Tom Lane
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)

2010-09-13 Thread Stefan Kaltenbrunner

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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Alvaro Herrera
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)

2010-09-13 Thread Greg Smith

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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Joshua D. Drake
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)

2010-09-13 Thread Thom Brown
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

2010-09-13 Thread Tom Lane
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)

2010-09-13 Thread Stefan Kaltenbrunner

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)

2010-09-13 Thread Greg Smith

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?

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Tom Lane
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

2010-09-13 Thread Teodor Sigaev

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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Hitoshi Harada
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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Robert Haas
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 Thread KaiGai Kohei

(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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Robert Haas
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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Thom Brown
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

2010-09-13 Thread Thom Brown
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-09-13 Thread Robert Haas
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

2010-09-13 Thread Thom Brown
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

2010-09-13 Thread Heikki Linnakangas
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

2010-09-13 Thread Fujii Masao
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)

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Heikki Linnakangas

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

2010-09-13 Thread Heikki Linnakangas

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?

2010-09-13 Thread Michael Haggerty

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