[HACKERS] Function to get size of notification queue?

[Brendan Jurd]

Hi folks,

I have a project which uses Postgres asynchronous notifications pretty
heavily.  It has a particularly Fun failure mode which causes the
notification queue to fill up.  To better debug this problem I'd like
to be able to monitor the size of the notification queue over time.

It doesn't look like we have a pg_notify_queue_size() or equivalent.
Should we?  Or would I be better off just watching the size of the
pg_notify/ directory on disk?

Cheers,
BJ


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Review of "pg_basebackup and pg_receivexlog to use non-blocking socket communication", was: Re: [HACKERS] Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown

[Hari Babu]

On January 01, 2013 10:19 PM Boszormenyi Zoltan wrote:
>I am reviewing your patch.
>• Is the patch in context diff format? 
>Yes.

Thanks for reviewing the patch.

>• Does it apply cleanly to the current git master?
>Not quite cleanly but it doesn't produce rejects or fuzz, only offset
warnings:

Will rebase the patch to head.

>• Does it include reasonable tests, necessary doc patches, etc? 
>The test cases are not applicable. There is no test framework for
>testing network outage in "make check".
>
>There are no documentation patches for the new --recvtimeout=INTERVAL
>and --conntimeout=INTERVAL options for either pg_basebackup or
>pg_receivexlog.

I will add the documentation for the same.


>Per the previous comment, no. But those are for the backend
>to notice network breakdowns and as such, they need a
>separate patch. 

I also think it is better to handle it as a separate patch for walsender.

>• Are the comments sufficient and accurate?
>This chunk below removes a comment which seems obvious enough
>so it's not needed:
>***
>*** 518,524  ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos,
uint32 timeline,
>    goto error;
>    }
>  
>!   /* Check the message type. */
>    if (copybuf[0] == 'k')
>    {
>    int pos;
>--- 559,568 
>    goto error;
>    }
>  
>!   /* Set the last reply timestamp */
>!   last_recv_timestamp = localGetCurrentTimestamp();
>!   ping_sent = false;
>!   
>    if (copybuf[0] == 'k')
>    {
>    int pos;
>***
>
>Other comments are sufficient and accurate.

I will fix and update the patch.

Please let me know if anything apart from above needs to be taken care.

Regards,
Hari babu.




-- 
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] Make pg_basebackup configure and start standby [Review]

[Tom Lane]

Boszormenyi Zoltan  writes:
> 2013-01-01 17:18 keltezéssel, Magnus Hagander írta:
>> That way we can get around the whole need for changing memory allocation 
>> across all the 
>> frontends, no? Like the attached.

> Sure it's simpler but then the consistent look of the code is lost.

> What about the other patch to unify pg_malloc and friends?
> Basically all client code boils down to
>  fprintf(stderr, ...)
> in different disguise in their error reporting, so that patch can
> also be simplified but it seems that the atexit() - either explicitly
> or hidden behind InitPostgresFrontend() - cannot be avoided.

Meh.  I find it seriously wrongheaded that something as minor as an
escape_quotes() function should get to dictate both malloc wrappers
and error recovery handling throughout every program that might use it.
I like Magnus' version a lot better than that idea.

A bigger issue that I notice with this code is that it's only correct in
backend-safe encodings, as the comment mentions.  If we're going to be
putting it into frontend programs, how safe is that going to be?

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] default SSL compression (was: libpq compression)

[Tom Lane]

Noah Misch  writes:
> On Tue, Jan 01, 2013 at 04:29:35PM +0100, Magnus Hagander wrote:
>> On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian  wrote:
>>> Do we want to change our ssl_ciphers default to 'DEFAULT'?  Currently it
>>> is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.

>> Did we ever get anywhere with this? Is this a change we want to do for 9.3?
>> Since nobody seems to have come up with a motivation for not following the
>> openssl default, we probably should?

> +1 for doing that.  I'm not aware of a PostgreSQL-specific selection criterion
> for SSL cipher suites.

I did a bit of digging in the commit logs.  The current default was
introduced in commit 17386ac45345fe38a10caec9d6e63afa3ce31bb9.  So far
as I can find, the only discussion leading up to that patch was the
thread starting at
http://archives.postgresql.org/pgsql-interfaces/2003-04/msg00075.php
which only discusses the key renegotiation interval, not anything about
selecting the allowed ciphers.  What's more, one might be forgiven for
suspecting that the cipherset string wasn't too carefully researched
after noticing that it wasn't even spelled correctly in that commit.

So +1 for changing it to "DEFAULT" from me, too.  There's no reason to
think we know more about this than the OpenSSL authors.

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] dynamic SQL - possible performance regression in 9.2

[Tom Lane]

I wrote:
> I'm inclined to think that Heikki's patch doesn't go far enough, if we
> want to optimize behavior in this case.  What we really want to happen
> is that parsing, planning, and execution all happen in the caller's
> memory context, with no copying of parse or plan trees at all - and we
> could do without overhead such as dependency extraction and invalidation
> checking, too.  This would make SPI_execute a lot more comparable to the
> behavior of exec_simple_query().

Here's a draft patch for that.  My initial hack at it had a
disadvantage, which was that because no invalidation checking happened,
a SPI_execute query string containing a DDL command (such as ALTER TABLE)
followed by a command affected by the DDL would fail to reparse/replan
the second command properly.  (I suspect that Heikki's version had a
related defect, but haven't looked closely.)  Now that's not a huge deal
IMO, because in many common cases parse analysis of the second command
would fail anyway.  For instance, this has never worked in any PG
release:

do $$ begin execute 'create table foo(f1 int); insert into foo values(1);'; end 
$$;

However it troubled me that there might be some regression there, and
after a bit of reflection I decided the right fix would be to rearrange
the code in spi.c so that parse analysis of later parsetrees follows
execution of earlier ones.  This makes the behavior of SPI_execute()
even more like that of exec_simple_query(), and shouldn't cost anything
noticeable given the other changes here.

I'm not entirely sure about performance of this fix, though.  I got
numbers varying between roughly-on-par with 9.1 and 10% slower than 9.1
for Pavel's example, depending on seemingly not-performance-related
rearrangements of the code in spi.c.  I think this must be chance
effects of cache line alignment, but it would be good to hear what other
people get, both on Pavel's example and the other ones alluded to.
In any case this seems better than unmodified HEAD, which was 40% slower
than 9.1 for me.

regards, tom lane

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 416a2c4f3bbe2132ed6d66f6eeaee9067e915d9b..cb5b84509ce8a89dfa8bf309c9c64ffdfdffe43d 100644
*** a/src/backend/executor/spi.c
--- b/src/backend/executor/spi.c
*** static int	_SPI_curid = -1;
*** 49,56 
  static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
  		 ParamListInfo paramLI, bool read_only);
  
! static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan,
!   ParamListInfo boundParams);
  
  static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
    Snapshot snapshot, Snapshot crosscheck_snapshot,
--- 49,57 
  static Portal SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
  		 ParamListInfo paramLI, bool read_only);
  
! static void _SPI_prepare_plan(const char *src, SPIPlanPtr plan);
! 
! static void _SPI_prepare_oneshot_plan(const char *src, SPIPlanPtr plan);
  
  static int _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
    Snapshot snapshot, Snapshot crosscheck_snapshot,
*** SPI_execute(const char *src, bool read_o
*** 355,361 
  	plan.magic = _SPI_PLAN_MAGIC;
  	plan.cursor_options = 0;
  
! 	_SPI_prepare_plan(src, &plan, NULL);
  
  	res = _SPI_execute_plan(&plan, NULL,
  			InvalidSnapshot, InvalidSnapshot,
--- 356,362 
  	plan.magic = _SPI_PLAN_MAGIC;
  	plan.cursor_options = 0;
  
! 	_SPI_prepare_oneshot_plan(src, &plan);
  
  	res = _SPI_execute_plan(&plan, NULL,
  			InvalidSnapshot, InvalidSnapshot,
*** SPI_execute_with_args(const char *src,
*** 506,512 
  	paramLI = _SPI_convert_params(nargs, argtypes,
    Values, Nulls);
  
! 	_SPI_prepare_plan(src, &plan, paramLI);
  
  	res = _SPI_execute_plan(&plan, paramLI,
  			InvalidSnapshot, InvalidSnapshot,
--- 507,513 
  	paramLI = _SPI_convert_params(nargs, argtypes,
    Values, Nulls);
  
! 	_SPI_prepare_oneshot_plan(src, &plan);
  
  	res = _SPI_execute_plan(&plan, paramLI,
  			InvalidSnapshot, InvalidSnapshot,
*** SPI_prepare_cursor(const char *src, int 
*** 547,553 
  	plan.parserSetup = NULL;
  	plan.parserSetupArg = NULL;
  
! 	_SPI_prepare_plan(src, &plan, NULL);
  
  	/* copy plan to procedure context */
  	result = _SPI_make_plan_non_temp(&plan);
--- 548,554 
  	plan.parserSetup = NULL;
  	plan.parserSetupArg = NULL;
  
! 	_SPI_prepare_plan(src, &plan);
  
  	/* copy plan to procedure context */
  	result = _SPI_make_plan_non_temp(&plan);
*** SPI_prepare_params(const char *src,
*** 584,590 
  	plan.parserSetup = parserSetup;
  	plan.parserSetupArg = parserSetupArg;
  
! 	_SPI_prepare_plan(src, &plan, NULL);
  
  	/* copy plan to procedure context */
  	result = _SPI_make_plan_non_temp(&plan);
--- 585,591 
  	plan.parserSetup = parserSetup;
  	plan.parserSetupArg = parserSetupArg;
  
! 	_SPI_prepare_plan(src, &pla

Re: [HACKERS] default SSL compression (was: libpq compression)

[Noah Misch]

On Tue, Jan 01, 2013 at 04:29:35PM +0100, Magnus Hagander wrote:
> On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian  wrote:
> > On Sun, Jun 17, 2012 at 11:45:54PM +0800, Magnus Hagander wrote:
> > > Uh. We have the ! notation in our default *now*. What openssl also
> > > supports is the text "DEFAULT", which is currently the equivalent of
> > > "ALL!aNULL!eNULL". The question, which is valid of course, should be
> > > if "DEFAULT" works with all openssl versions.
> > >
> > > It would seem reasonable it does, but I haven't investigated.

The oldest version readily available for download (0.9.1c, 1998) has it.

> > Do we want to change our ssl_ciphers default to 'DEFAULT'?  Currently it
> > is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.
> >
> Did we ever get anywhere with this? Is this a change we want to do for 9.3?
> Since nobody seems to have come up with a motivation for not following the
> openssl default, we probably should?

+1 for doing that.  I'm not aware of a PostgreSQL-specific selection criterion
for SSL cipher suites.


-- 
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] Make pg_basebackup configure and start standby [Review]

[Boszormenyi Zoltan]

2013-01-01 17:18 keltezéssel, Magnus Hagander írta:


On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan > wrote:


Hi,

now that PQconninfo() was committed, I rebased the subsequent
patches. Actual code change was only in the last patch, to
conform to the committed PQconninfo() API.


So, finally coming back to this one.

What happened to Alvaro's suggestion of:

> It seems simpler to have the escape_quotes utility function in port just
> not use pg_malloc at all, have it return NULL or similar failure
> indicator when malloc() fails, and then the caller decides what to do.


I seem to have skipped it while reading my mails,
I don't remember this suggestion at all. Sorry.



That way we can get around the whole need for changing memory allocation across all the 
frontends, no? Like the attached.


Sure it's simpler but then the consistent look of the code is lost.

What about the other patch to unify pg_malloc and friends?
Basically all client code boils down to
fprintf(stderr, ...)
in different disguise in their error reporting, so that patch can
also be simplified but it seems that the atexit() - either explicitly
or hidden behind InitPostgresFrontend() - cannot be avoided.

Best regards,
Zoltán Böszörményi

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/

Re: [HACKERS] [PATCH] Make pg_basebackup configure and start standby [Review]

[Magnus Hagander]

On Tue, Jan 1, 2013 at 7:13 PM, Boszormenyi Zoltan  wrote:

>  2013-01-01 18:25 keltezéssel, Magnus Hagander írta:
>
>
> On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan wrote:
>
>> Hi,
>>
>> now that PQconninfo() was committed, I rebased the subsequent
>> patches. Actual code change was only in the last patch, to
>> conform to the committed PQconninfo() API.
>>
>>
>  I've applied a modified version of the "tar unification" patch to
> master. It didn't apply cleanly so I ended up redoing a number of the
> things manually, but the end result is fairly similar. I don't think it'll
> cause anything but really trivial merge conflicts against the 04 patch, but
> I didn't actually check that (e.g. it's tarCreateHeader() not
> _tarCreateHeader() now).
>
>
> Thanks.
>
>
>I took at look at the basebackup patch as well. Easier to get now that
> it's commented :) There's quite a lot of code in there whereby it tries to
> remove recovery.conf from the tar stream if it's already in there. That
> seems like an ugly way to do it. I see two other options, that I think both
> are better. If we do need it to be removed, we should probably pass that as
> a parameter up to the walsender, and make sure the file isn't included in
> the first place. But we can also leave it in there - the tar format is
> perfectly happy to have multiple copies of the same file in the archive -
> it'll just use whichever copy comes last.
>
>
> IIRC, Fujii Masao's wish was to remove the recovery.conf from the tar
> stream.
> I know that the tar format is perfectly happy with two files with the same
> path name but his reasoning was that GUIs (like WinRar) may get confused
> by two such files and cannot decide which one to extract. I didn't actually
> test this but it is a reasonable suspicion.
>

Hmm. Somehow, I missed that part of the discussion. Sorry, didn't realize
it had been mentioned already.


Passing a parameter to the walsender may be a better solution.
> It's a bad idea only because it wasn't mine. ;-)
>
> The only problem with this idea is that currently there's nothing that
> stops
> pg_basebackup to be a generic and backwards-compatible tool.
> It simply receives a TAR stream via plain PQgetCopyData() and optionally
> extracts it. The client-side solution to skip the recovery.conf file keeps
> this
> backward compatible feature. I tested this and pg_basebackup from 9.3dev
> happily backs up a 9.2.2 database...
>

I thought commit add6c3179a4d4fa3e62dd3e86a00f23303336bac at leasdt broke
that? In particular, did you test where any of those values were different
between client and server? Or maybe just didn't test in -x mode?

I actually thought there was something else that broke the compatibility,
but I can't seem to find it so perhaps that was something that was never
committed.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] dynamic SQL - possible performance regression in 9.2

[Tom Lane]

Peter Eisentraut  writes:
> On 12/28/12 5:11 PM, Heikki Linnakangas wrote:
>> It looks like the regression is caused by extra copying of the parse
>> tree and plan trees. Node-copy-related functions like AllocSetAlloc and
>> _copy* are high in the profile, They are also high in the 9.1 profile,
>> but even more so in 9.2.

Hm ... those 9.2 changes were supposed to *improve* performance, and I
believe they did so for code paths where the plan cache is actually
doing something useful.  In this path, it's basically useless.

>> I hacked together a quick&dirty patch to reduce the copying of
>> single-shot plans, and was able to buy back much of the regression I was
>> seeing on DBT-2. Patch attached. But of course, DBT-2 really should be
>> preparing the queries once with SPI_prepare, and reusing them thereafter.

> I was recently profiling an application that uses a fair amount of
> PL/pgSQL with dynamic queries and also noticed AllocSetAlloc high in the
> profile.  I was getting suspicious now and compared 9.1 and 9.2
> performance: 9.2 is consistently about 3% slower.  Your patch doesn't
> seem to have a measurable effect, but it might be if I ran the test for
> longer.

I'm inclined to think that Heikki's patch doesn't go far enough, if we
want to optimize behavior in this case.  What we really want to happen
is that parsing, planning, and execution all happen in the caller's
memory context, with no copying of parse or plan trees at all - and we
could do without overhead such as dependency extraction and invalidation
checking, too.  This would make SPI_execute a lot more comparable to the
behavior of exec_simple_query().

So basically plancache.c has got no useful functionality to offer here.

But to avoid having multiple drastically different code paths in spi.c,
it would be nice if we had some sort of "shell" API that would provide
the illusion of using a CachedPlan without any of the overhead that
comes along with actually being able to save or reuse the plan.
Heikki's "oneshot" concept is moving in that direction, but not far
enough IMO.  I'm thinking we don't want it to create any new memory
contexts at all, just palloc a CachedPlan object directly in the
caller's memory context and link to the caller-supplied trees.

I'll take a whack at 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] [PATCH] Make pg_basebackup configure and start standby [Review]

[Boszormenyi Zoltan]

2013-01-01 18:25 keltezéssel, Magnus Hagander írta:


On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan > wrote:


Hi,

now that PQconninfo() was committed, I rebased the subsequent
patches. Actual code change was only in the last patch, to
conform to the committed PQconninfo() API.


I've applied a modified version of the "tar unification" patch to master. It didn't 
apply cleanly so I ended up redoing a number of the things manually, but the end result 
is fairly similar. I don't think it'll cause anything but really trivial merge conflicts 
against the 04 patch, but I didn't actually check that (e.g. it's tarCreateHeader() not 
_tarCreateHeader() now).


Thanks.

I took at look at the basebackup patch as well. Easier to get now that it's commented :) 
There's quite a lot of code in there whereby it tries to remove recovery.conf from the 
tar stream if it's already in there. That seems like an ugly way to do it. I see two 
other options, that I think both are better. If we do need it to be removed, we should 
probably pass that as a parameter up to the walsender, and make sure the file isn't 
included in the first place. But we can also leave it in there - the tar format is 
perfectly happy to have multiple copies of the same file in the archive - it'll just use 
whichever copy comes last.


IIRC, Fujii Masao's wish was to remove the recovery.conf from the tar stream.
I know that the tar format is perfectly happy with two files with the same
path name but his reasoning was that GUIs (like WinRar) may get confused
by two such files and cannot decide which one to extract. I didn't actually
test this but it is a reasonable suspicion.

Passing a parameter to the walsender may be a better solution.
It's a bad idea only because it wasn't mine. ;-)

The only problem with this idea is that currently there's nothing that stops
pg_basebackup to be a generic and backwards-compatible tool.
It simply receives a TAR stream via plain PQgetCopyData() and optionally
extracts it. The client-side solution to skip the recovery.conf file keeps this
backward compatible feature. I tested this and pg_basebackup from 9.3dev
happily backs up a 9.2.2 database...



Given that the code there is already fairly difficult to track, I think just keeping it 
simpler is definitely worth doing one of those two. I would vote for just leaving two 
copies of recovery.conf in there.


Comments/thoughts from others?

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Best regards,
Zoltán Böszörmény

--
--
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
 http://www.postgresql.at/

Re: [HACKERS] [PATCH] Make pg_basebackup configure and start standby [Review]

[Magnus Hagander]

On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan  wrote:

> Hi,
>
> now that PQconninfo() was committed, I rebased the subsequent
> patches. Actual code change was only in the last patch, to
> conform to the committed PQconninfo() API.
>
>
I've applied a modified version of the "tar unification" patch to master.
It didn't apply cleanly so I ended up redoing a number of the things
manually, but the end result is fairly similar. I don't think it'll cause
anything but really trivial merge conflicts against the 04 patch, but I
didn't actually check that (e.g. it's tarCreateHeader() not
_tarCreateHeader() now).


I took at look at the basebackup patch as well. Easier to get now that it's
commented :) There's quite a lot of code in there whereby it tries to
remove recovery.conf from the tar stream if it's already in there. That
seems like an ugly way to do it. I see two other options, that I think both
are better. If we do need it to be removed, we should probably pass that as
a parameter up to the walsender, and make sure the file isn't included in
the first place. But we can also leave it in there - the tar format is
perfectly happy to have multiple copies of the same file in the archive -
it'll just use whichever copy comes last.

Given that the code there is already fairly difficult to track, I think
just keeping it simpler is definitely worth doing one of those two. I would
vote for just leaving two copies of recovery.conf in there.

Comments/thoughts from others?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] pg_retainxlog for inclusion in 9.3?

[Dimitri Fontaine]

Magnus Hagander  writes:
> Is this a tool that people would like to see included in the general
> toolchain? If so, I'll reformat it to work in the general build
> environment and submit it for the last commitfest.

Please do.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Review of "pg_basebackup and pg_receivexlog to use non-blocking socket communication", was: Re: [HACKERS] Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown

[Boszormenyi Zoltan]

Hi,

2012-11-15 14:59 keltezéssel, Amit kapila írta:

On Monday, November 12, 2012 8:23 PM Fujii Masao wrote:
On Fri, Nov 9, 2012 at 3:03 PM, Amit Kapila  wrote:

On Thursday, November 08, 2012 10:42 PM Fujii Masao wrote:

On Thu, Nov 8, 2012 at 5:53 PM, Amit Kapila 
wrote:

On Thursday, November 08, 2012 2:04 PM Heikki Linnakangas wrote:

On 19.10.2012 14:42, Amit kapila wrote:

On Thursday, October 18, 2012 8:49 PM Fujii Masao wrote:

Are you planning to introduce the timeout mechanism in pg_basebackup
main process? Or background process? It's useful to implement both.

By background process, you mean ReceiveXlogStream?
For both.
I think for background process, it can be done in a way similar to what we
have done for walreceiver.

Yes.

But I have some doubts for how to do for main process:
Logic similar to walreceiver can not be used incase network goes down during
getting other database file from server.
The reason for the same is to receive the data files PQgetCopyData() is
called in synchronous mode, so it keeps waiting for infinite time till it
gets some data.
In order to solve this issue, I can think of following options:
1. Making this call also asynchronous (but now sure about impact of this).

+1
Walreceiver already calls PQgetCopyData() asynchronously. ISTM you can
solve the issue in the similar way to walreceiver's.

2. In function pqWait, instead of passing hard-code value -1 (i.e. infinite
wait), we can send some finite time. This time can be received as command
line argument
 from respective utility and set the same in PGconn structure.

Yes, I think that we should add something like --conninfo option to
pg_basebackup
and pg_receivexlog. We can easily set not only connect_timeout but also sslmode,
application_name, ... by using such option accepting conninfo string.

I have prepared an attached patch to make pg_basebackup and pg_receivexlog as 
non-blocking.
To do so I have to add new command line parameters in pg_basebackup and 
pg_receivexlog
for now added two more command line arguments
 a.  "-r"  for pg_basebackup and pg_receivexlog to take receive 
time-out value. Default value for this parameter is 60 sec.
 b. "-t"   for pg_basebackup and pg_receivexlog to take initial 
connection timeout value. Default value is infinite wait.
We can change to accept --conninfo as well.

I feel apart from above, remaining problem is for function call PQgetResult()
1. Wherever query is getting sent from BaseBackup, it calls the function 
PQgetResult to receive the result of query.
 As PQgetResult() is blocking function (it calls pqWait which can hang), so 
if network is down before sending the query itself,
 then there will not be any result, so it will keep hanging in PQgetResult .
IMO, it can be solved in below ways:
a. Create one corresponding non-blocking function. But this function is being 
called from inside some of the
  other libpq function (PQexec->PQexecFinish->PQgetResult). So it can be 
little tricky to solve this way.
b. Add the receive_timeout variable in PGconn structure and use it in pqWait 
for timeout whenever it is set.
c. any other better way?



BTW, IIRC the walsender has no timeout mechanism during sending
backup data to pg_basebackup. So it's also useful to implement the
timeout mechanism for the walsender during backup.

What about using pq_putmessage_noblock()?

I think may be some more functions also needs to be made as noblock. I am still 
evaluating.

I will upload the attached patch in commitfest if you don't have any objections?

More Suggestions/Comments?

With Regards,
Amit Kapila.


I am reviewing your patch.

 * Is the patch in context diff format 
?


Yes.

 * Does it apply cleanly to the current git master?


Not quite cleanly but it doesn't produce rejects or fuzz, only offset warnings:

[zozo@localhost postgresql]$ cat ../noblock_basebackup_and_receivexlog.patch | 
patch -p1
patching file src/bin/pg_basebackup/pg_basebackup.c
Hunk #1 succeeded at 41 (offset -6 lines).
Hunk #2 succeeded at 123 (offset -6 lines).
Hunk #3 succeeded at 239 (offset -6 lines).
Hunk #4 succeeded at 292 (offset -6 lines).
Hunk #5 succeeded at 470 (offset -6 lines).
Hunk #6 succeeded at 588 (offset -6 lines).
Hunk #7 succeeded at 601 (offset -6 lines).
Hunk #8 succeeded at 727 (offset -6 lines).
Hunk #9 succeeded at 779 (offset -6 lines).
Hunk #10 succeeded at 797 (offset -6 lines).
Hunk #11 succeeded at 811 (offset -6 lines).
Hunk #12 succeeded at 879 (offset -6 lines).
Hunk #13 succeeded at 1080 (offset -6 lines).
Hunk #14 succeeded at 1381 (offset -6 lines).
Hunk #15 succeeded at 1409 (offset -6 lines).
Hunk #16 succeeded at 1521 (offset -6 lines).
patching file src/bin/pg_basebackup/pg_receivexlog.c
Hunk #1 succeeded at 35 (offset -6 lines).
Hunk #2 succeeded at 65 (offset -6 lines).
Hunk #3 succeeded at 224 (offset -6 lines).
Hunk #4 succeeded at 281 (offset -6 lines).
Hunk #5 succeeded at 314 (offset 

Re: [HACKERS] PATCH: optimized DROP of multiple tables within a transaction

[Tomas Vondra]

On 1.1.2013 17:35, Alvaro Herrera wrote:
> There was an earlier suggestion by Andres Freund to use memcmp()
> instead, but I don't see that in the latest posted version of the patch;
> was there a specific rationale for taking it out or it was just lost in
> the shuffle?

No, I've tried that approach with a comparator like this:

static int
rnode_comparator(const void * p1, const void * p2)
{
return memcmp(p1, p2, sizeof(RelFileNode));
}

but it turned out to be slower than the current comparator. I've posted
some benchmark results and possible explanation on 20/12 (message
50d26fe8.1040...@fuzzy.cz).

If you could verify my results, that'd be great.

Tomas


-- 
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: optimized DROP of multiple tables within a transaction

[Alvaro Herrera]

There was an earlier suggestion by Andres Freund to use memcmp()
instead, but I don't see that in the latest posted version of the patch;
was there a specific rationale for taking it out or it was just lost in
the shuffle?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
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] Make pg_basebackup configure and start standby [Review]

[Magnus Hagander]

On Fri, Nov 30, 2012 at 10:13 AM, Boszormenyi Zoltan  wrote:

> Hi,
>
> now that PQconninfo() was committed, I rebased the subsequent
> patches. Actual code change was only in the last patch, to
> conform to the committed PQconninfo() API.


So, finally coming back to this one.

What happened to Alvaro's suggestion of:

> It seems simpler to have the escape_quotes utility function in port just
> not use pg_malloc at all, have it return NULL or similar failure
> indicator when malloc() fails, and then the caller decides what to do.

That way we can get around the whole need for changing memory allocation
across all the frontends, no? Like the attached.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


quotes.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] [PATCH] Change Windows build docs to point to flex and bison from msys

[Heikki Linnakangas]

On 31.12.2012 08:53, Craig Ringer wrote:

Hi all

For some time it's been impossible to build PostgreSQL on 64-bit Windows
by following the documentation's advice, as the version of Flex we
distribute on the PostgreSQL FTP site does not work on 64-bit Windows
hosts. See this 2011 message (
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00100.php).

The attached patch changes the documentation to point users to the
working flex and bison provided by msys - either as part of MinGW, or
from msysgit. It also mentions the error that people will get when
trying to use the old flex we distribute on 64-bit hosts so it's easier
to find out about the issue.


Thanks, committed. I left out the sentence about vcvarsall, I think 
that's beyond what we need to explain in our docs. Plus so


- Heikki


--
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_retainxlog for inclusion in 9.3?

[Hannu Krosing]

On 01/01/2013 04:10 PM, Magnus Hagander wrote:

So, it turns out the reason I got no feedback on this tool, was that I
forgot both to email about and to actually push the code to github :O
So this is actually code that's almost half a year old and that I was
supposed to submit for the first or second commitfest. Oops.

So, the tool and a README for it right now can be found at
https://github.com/mhagander/pg_retainxlog for the time being.

The idea behind the tool is to plug a hole in the case when
pg_receivexlog is used for log archiving, which is that you still need
a "blocking" archive_command in order to make sure that files aren't
recycled on the master. So for 9.2 you can do this with an
archive_command that checks if the file has appeared properly on the
slave - but that usually means you're back at requiring ssh
connectivity between the machines, for example. Even though this
information is actually avialable on the master...

This can also be of use to pure replication scenarios, where you don't
know how to tune wal_keep_segments, but using actual live feedback
instead of guessing.

When pg_retainxlog is used as an archive_command, it will check the
pg_stat_replication view instead of checking the slave. It will then
only return ok once the requested logfile has been replicated to the
slave. By default it will look for a replication client named
pg_receivexlog, but it supports overriding the query with anything -
so you can say things like "needs to have arrived on at least two
replication slaves before we consider it archived". Or if used instead
of wal_keep_segmnets, needs to have arrived at *all* replication
slaves.

Is this a tool that people would like to see included in the general
toolchain? If so, I'll reformat it to work in the general build
environment and submit it for the last commitfest.



+1


Hannu Krosing


--
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] default SSL compression (was: libpq compression)

[Magnus Hagander]

On Thu, Aug 30, 2012 at 11:41 PM, Bruce Momjian  wrote:

> On Sun, Jun 17, 2012 at 11:45:54PM +0800, Magnus Hagander wrote:
> > On Sun, Jun 17, 2012 at 11:42 PM, Tom Lane  wrote:
> > > Magnus Hagander  writes:
> > >> Is there a reason why we don't have a parameter on the client
> > >> mirroring ssl_ciphers?
> > >
> > > Dunno, do we need one?  I am not sure what the cipher negotiation
> process
> > > looks like or which side has the freedom to choose.
> >
> > I haven't looked into the details, but it seems reasonable that
> > *either* side should be able to at least define a list of ciphers it
> > *doens't* want to talk with.
> >
> > Do we need it - well, it makes sense for the client to be able to say
> > "I won't trust 56-bit encryption" before it sends over the password,
> > imo..
> >
> >
> > >> That, or just have DEFAULT as being the default (which in current
> > >> openssl means ALL:!aNULL:!eNULL.
> > >
> > > If our default isn't the same as the underlying default, I have to
> > > question why not.
> >
> > Yeah, that's exaclty what I'm questioning here..
> >
> > >  But are you sure this "!" notation will work with
> > > all openssl versions?
> >
> > Uh. We have the ! notation in our default *now*. What openssl also
> > supports is the text "DEFAULT", which is currently the equivalent of
> > "ALL!aNULL!eNULL". The question, which is valid of course, should be
> > if "DEFAULT" works with all openssl versions.
> >
> > It would seem reasonable it does, but I haven't investigated.
>
> Do we want to change our ssl_ciphers default to 'DEFAULT'?  Currently it
> is 'ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH'.
>
>
Did we ever get anywhere with this? Is this a change we want to do for 9.3?
Since nobody seems to have come up with a motivation for not following the
openssl default, we probably should?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

[HACKERS] pg_retainxlog for inclusion in 9.3?

[Magnus Hagander]

So, it turns out the reason I got no feedback on this tool, was that I
forgot both to email about and to actually push the code to github :O
So this is actually code that's almost half a year old and that I was
supposed to submit for the first or second commitfest. Oops.

So, the tool and a README for it right now can be found at
https://github.com/mhagander/pg_retainxlog for the time being.

The idea behind the tool is to plug a hole in the case when
pg_receivexlog is used for log archiving, which is that you still need
a "blocking" archive_command in order to make sure that files aren't
recycled on the master. So for 9.2 you can do this with an
archive_command that checks if the file has appeared properly on the
slave - but that usually means you're back at requiring ssh
connectivity between the machines, for example. Even though this
information is actually avialable on the master...

This can also be of use to pure replication scenarios, where you don't
know how to tune wal_keep_segments, but using actual live feedback
instead of guessing.

When pg_retainxlog is used as an archive_command, it will check the
pg_stat_replication view instead of checking the slave. It will then
only return ok once the requested logfile has been replicated to the
slave. By default it will look for a replication client named
pg_receivexlog, but it supports overriding the query with anything -
so you can say things like "needs to have arrived on at least two
replication slaves before we consider it archived". Or if used instead
of wal_keep_segmnets, needs to have arrived at *all* replication
slaves.

Is this a tool that people would like to see included in the general
toolchain? If so, I'll reformat it to work in the general build
environment and submit it for the last commitfest.

(comments on the code itself are of course also welcome)

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