Re: [HACKERS] Better auth errors from libpq

[Heikki Linnakangas]

Joshua D. Drake wrote:

David Fetter wrote:

On Thu, Sep 11, 2008 at 11:28:36PM -0400, Tom Lane wrote:

Joshua Drake <[EMAIL PROTECTED]> writes:

I think something like:
psql: FATAL:  Ident authentication failed for user "root"
HINT: 
http://www.postgresql.org/docs/8.3/static/client-authentication.html

Would be nice.

Do you really think that's helpful in the typical case where someone
fat-fingered their password?

I'm not averse to hint messages that are actually helpful, ie,
reasonably connected to the *specific* failure situation.  The problem
I've got with David's proposal is that it provides a one-size-fits-all
hint for every possible auth failure.  One size does not fit all here.


Here's a few different sizes: one for each auth method.


The only thing I would say here is that you point the URL to current 
which will be wrong in one release. Perhaps something that pulls the 
pgversion macro?


We don't put URLs in error messages. The hint needs to be a real sentence.

--
  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] Better auth errors from libpq

[Joshua D. Drake]

David Fetter wrote:

On Thu, Sep 11, 2008 at 11:28:36PM -0400, Tom Lane wrote:

Joshua Drake <[EMAIL PROTECTED]> writes:

I think something like:
psql: FATAL:  Ident authentication failed for user "root"
HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html
Would be nice.

Do you really think that's helpful in the typical case where someone
fat-fingered their password?

I'm not averse to hint messages that are actually helpful, ie,
reasonably connected to the *specific* failure situation.  The problem
I've got with David's proposal is that it provides a one-size-fits-all
hint for every possible auth failure.  One size does not fit all here.


Here's a few different sizes: one for each auth method.


The only thing I would say here is that you point the URL to current 
which will be wrong in one release. Perhaps something that pulls the 
pgversion macro?


Sincerely,

Joshua D. Drake




Cheers,
David.




--
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] Better auth errors from libpq

[Joshua D. Drake]

Tom Lane wrote:

Joshua Drake <[EMAIL PROTECTED]> writes:

I think something like:



psql: FATAL:  Ident authentication failed for user "root"
HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html



Would be nice.


Do you really think that's helpful in the typical case where someone
fat-fingered their password?


No but I do think it is helpful based on the error provided.



I'm not averse to hint messages that are actually helpful, ie,
reasonably connected to the *specific* failure situation.  The problem
I've got with David's proposal is that it provides a one-size-fits-all
hint for every possible auth failure.  One size does not fit all here.


I can agree with that.



Or to put it even more baldly: this is not an area in which you can
improve matters significantly with five minutes' thought and a one-line
patch.  It would take some actual work.


Fair enough.

Joshua D. Drake



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] Better auth errors from libpq

[David Fetter]

On Thu, Sep 11, 2008 at 11:28:36PM -0400, Tom Lane wrote:
> Joshua Drake <[EMAIL PROTECTED]> writes:
> > I think something like:
> 
> > psql: FATAL:  Ident authentication failed for user "root"
> > HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html
> 
> > Would be nice.
> 
> Do you really think that's helpful in the typical case where someone
> fat-fingered their password?
> 
> I'm not averse to hint messages that are actually helpful, ie,
> reasonably connected to the *specific* failure situation.  The problem
> I've got with David's proposal is that it provides a one-size-fits-all
> hint for every possible auth failure.  One size does not fit all here.

Here's a few different sizes: one for each auth method.

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Index: src/backend/libpq/auth.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.167
diff -c -r1.167 auth.c
*** src/backend/libpq/auth.c1 Aug 2008 11:41:12 -   1.167
--- src/backend/libpq/auth.c12 Sep 2008 03:51:29 -
***
*** 197,202 
--- 197,203 
  auth_failed(Port *port, int status)
  {
const char *errstr;
+   const char *hintstr;
  
/*
 * If we failed due to EOF from client, just quit; there's no point in
***
*** 215,259 
{
case uaReject:
errstr = gettext_noop("authentication failed for user 
\"%s\": host rejected");
break;
case uaKrb5:
errstr = gettext_noop("Kerberos 5 authentication failed 
for user \"%s\"");
break;
case uaGSS:
errstr = gettext_noop("GSSAPI authentication failed for 
user \"%s\"");
break;
case uaSSPI:
errstr = gettext_noop("SSPI authentication failed for 
user \"%s\"");
break;
case uaTrust:
errstr = gettext_noop("\"trust\" authentication failed 
for user \"%s\"");
break;
case uaIdent:
errstr = gettext_noop("Ident authentication failed for 
user \"%s\"");
break;
case uaMD5:
case uaCrypt:
case uaPassword:
errstr = gettext_noop("password authentication failed 
for user \"%s\"");
break;
  #ifdef USE_PAM
case uaPAM:
errstr = gettext_noop("PAM authentication failed for 
user \"%s\"");
break;
  #endif   /* USE_PAM */
  #ifdef USE_LDAP
case uaLDAP:
errstr = gettext_noop("LDAP authentication failed for 
user \"%s\"");
break;
  #endif   /* USE_LDAP */
default:
errstr = gettext_noop("authentication failed for user 
\"%s\": invalid authentication method");
break;
}
  
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
!errmsg(errstr, port->user_name)));
/* doesn't return */
  }
  
--- 216,271 
{
case uaReject:
errstr = gettext_noop("authentication failed for user 
\"%s\": host rejected");
+   hintstr = 
gettext_noop("http://www.postgresql.org/docs/current/static/auth-methods.html";);
break;
case uaKrb5:
errstr = gettext_noop("Kerberos 5 authentication failed 
for user \"%s\"");
+   hintstr = 
gettext_noop("http://www.postgresql.org/docs/current/static/auth-methods.html#GSSAPI-AUTH";);
break;
case uaGSS:
errstr = gettext_noop("GSSAPI authentication failed for 
user \"%s\"");
+   hintstr = 
gettext_noop("http://www.postgresql.org/docs/current/static/auth-methods.html#GSSAPI-AUTH";);
break;
case uaSSPI:
errstr = gettext_noop("SSPI authentication failed for 
user \"%s\"");
+   hintstr = 
gettext_noop("http://www.postgresql.org/docs/current/static/auth-methods.html#SSPI-AUTH";);
break;
case uaTrust:
errstr = gettext_noop("\"trust\" authentication failed 
for user \"%s\"");
+   hintstr = 
gettext_noop("http://www.postgresql.org/docs/current/static/auth-methods.html#TRUST-AUT

Re: [HACKERS] Better auth errors from libpq

[David Fetter]

On Thu, Sep 11, 2008 at 11:28:36PM -0400, Tom Lane wrote:
> Joshua Drake <[EMAIL PROTECTED]> writes:
> > I think something like:
> 
> > psql: FATAL:  Ident authentication failed for user "root"
> > HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html
> 
> > Would be nice.
> 
> Do you really think that's helpful in the typical case where someone
> fat-fingered their password?

Possibly not.

> I'm not averse to hint messages that are actually helpful, ie,
> reasonably connected to the *specific* failure situation.  The
> problem I've got with David's proposal is that it provides a
> one-size-fits-all hint for every possible auth failure.  One size
> does not fit all here.

I'd be delighted to make more different sizes.

> Or to put it even more baldly: this is not an area in which you can
> improve matters significantly with five minutes' thought and a
> one-line patch.  It would take some actual work.

It's work that hasn't yet been done, and thanks for your input on the
first versions :)

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

Re: [HACKERS] Better auth errors from libpq

[Tom Lane]

Joshua Drake <[EMAIL PROTECTED]> writes:
> I think something like:

> psql: FATAL:  Ident authentication failed for user "root"
> HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html

> Would be nice.

Do you really think that's helpful in the typical case where someone
fat-fingered their password?

I'm not averse to hint messages that are actually helpful, ie,
reasonably connected to the *specific* failure situation.  The problem
I've got with David's proposal is that it provides a one-size-fits-all
hint for every possible auth failure.  One size does not fit all here.

Or to put it even more baldly: this is not an area in which you can
improve matters significantly with five minutes' thought and a one-line
patch.  It would take some actual work.

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] Better auth errors from libpq

[David Fetter]

On Thu, Sep 11, 2008 at 08:10:45PM -0700, Joshua D. Drake wrote:
> On Thu, 11 Sep 2008 22:59:40 -0400
> Tom Lane <[EMAIL PROTECTED]> wrote:
> 
> > > psql: FATAL:  Ident authentication failed for user "root"
> > > HINT:  Is pg_hba.conf set properly on the server?
> > 
> > Seems pretty useless.  What does "set properly" mean?  There isn't
> > even any good reason to think that the solution to most auth failures
> > is to change pg_hba.conf, so I'd bet that this hint is wrong far more
> > often than it's right.
> 
> I think something like:
> 
> psql: FATAL:  Ident authentication failed for user "root"
> HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html
> 
> Would be nice. Heck in most modern terminals the HINT would be
> clickable too :)

I'm all for something, and that's a much better something.  What we
have now--nothing--actively distresses newbies for no good reason.

I don't know how many people we've lost right at that point, but the
number has to be high, as most people don't just hop into IRC with
their problem.

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

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

Re: [HACKERS] Better auth errors from libpq

[Joshua Drake]

On Thu, 11 Sep 2008 22:59:40 -0400
Tom Lane <[EMAIL PROTECTED]> wrote:

> > psql: FATAL:  Ident authentication failed for user "root"
> > HINT:  Is pg_hba.conf set properly on the server?
> 
> Seems pretty useless.  What does "set properly" mean?  There isn't
> even any good reason to think that the solution to most auth failures
> is to change pg_hba.conf, so I'd bet that this hint is wrong far more
> often than it's right.

I think something like:

psql: FATAL:  Ident authentication failed for user "root"
HINT: http://www.postgresql.org/docs/8.3/static/client-authentication.html

Would be nice. Heck in most modern terminals the HINT would be
clickable too :)

Sincerely,

Joshua D. Drake
-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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

Re: [HACKERS] Better auth errors from libpq

[Tom Lane]

David Fetter <[EMAIL PROTECTED]> writes:
> This isn't exactly informative to newbies, so I'm proposing a patch
> like that attached for such failures.  Instead of seeing that
> mysterious message, they'd get something like this:

> psql: FATAL:  Ident authentication failed for user "root"
> HINT:  Is pg_hba.conf set properly on the server?

Seems pretty useless.  What does "set properly" mean?  There isn't even
any good reason to think that the solution to most auth failures is to
change pg_hba.conf, so I'd bet that this hint is wrong far more often
than it's right.

You have to recall also that we deliberately suppress details in auth
failure messages sent to the client, since they might provide useful
clues to someone trying to break in.  Admittedly, the above is so
content-free that it gives no aid or comfort to an attacker, but
I don't see that it provides any to a novice DBA either.

regards, tom lane

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

[HACKERS] Better auth errors from libpq

[David Fetter]

Folks,

Far and away the most common question we get in IRC includes phrases
like:

psql: FATAL:  Ident authentication failed for user "root"

This isn't exactly informative to newbies, so I'm proposing a patch
like that attached for such failures.  Instead of seeing that
mysterious message, they'd get something like this:

psql: FATAL:  Ident authentication failed for user "root"
HINT:  Is pg_hba.conf set properly on the server?

Also good would be some way to make man pages and whatever Windows
uses both for pg_hba.conf and for postgresql.conf.  Any hints as to
how to start this would be most welcome :)

Cheers,
David.
-- 
David Fetter <[EMAIL PROTECTED]> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
? GNUmakefile
? config.log
? config.status
? hint.diff
? src/Makefile.global
? src/backend/postgres
? src/backend/catalog/postgres.bki
? src/backend/catalog/postgres.description
? src/backend/catalog/postgres.shdescription
? src/backend/snowball/snowball_create.sql
? src/backend/utils/probes.h
? src/backend/utils/mb/conversion_procs/conversion_create.sql
? src/bin/initdb/initdb
? src/bin/pg_config/pg_config
? src/bin/pg_controldata/pg_controldata
? src/bin/pg_ctl/pg_ctl
? src/bin/pg_dump/pg_dump
? src/bin/pg_dump/pg_dumpall
? src/bin/pg_dump/pg_restore
? src/bin/pg_resetxlog/pg_resetxlog
? src/bin/psql/psql
? src/bin/scripts/clusterdb
? src/bin/scripts/createdb
? src/bin/scripts/createlang
? src/bin/scripts/createuser
? src/bin/scripts/dropdb
? src/bin/scripts/droplang
? src/bin/scripts/dropuser
? src/bin/scripts/reindexdb
? src/bin/scripts/vacuumdb
? src/include/pg_config.h
? src/include/stamp-h
? src/interfaces/ecpg/compatlib/exports.list
? src/interfaces/ecpg/compatlib/libecpg_compat.so.3.1
? src/interfaces/ecpg/ecpglib/exports.list
? src/interfaces/ecpg/ecpglib/libecpg.so.6.1
? src/interfaces/ecpg/include/ecpg_config.h
? src/interfaces/ecpg/pgtypeslib/exports.list
? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.3.1
? src/interfaces/ecpg/preproc/ecpg
? src/interfaces/libpq/exports.list
? src/interfaces/libpq/libpq.so.5.2
? src/port/pg_config_paths.h
? src/test/regress/log
? src/test/regress/pg_regress
? src/test/regress/results
? src/test/regress/testtablespace
? src/test/regress/tmp_check
? src/test/regress/expected/constraints.out
? src/test/regress/expected/copy.out
? src/test/regress/expected/create_function_1.out
? src/test/regress/expected/create_function_2.out
? src/test/regress/expected/largeobject.out
? src/test/regress/expected/largeobject_1.out
? src/test/regress/expected/misc.out
? src/test/regress/expected/tablespace.out
? src/test/regress/sql/constraints.sql
? src/test/regress/sql/copy.sql
? src/test/regress/sql/create_function_1.sql
? src/test/regress/sql/create_function_2.sql
? src/test/regress/sql/largeobject.sql
? src/test/regress/sql/misc.sql
? src/test/regress/sql/tablespace.sql
? src/timezone/zic
Index: src/backend/libpq/auth.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.167
diff -c -r1.167 auth.c
*** src/backend/libpq/auth.c1 Aug 2008 11:41:12 -   1.167
--- src/backend/libpq/auth.c12 Sep 2008 02:41:43 -
***
*** 197,202 
--- 197,204 
  auth_failed(Port *port, int status)
  {
const char *errstr;
+   const char *hintstr;
+   hintstr = gettext_noop("Is pg_hba.conf set properly on the server?");
  
/*
 * If we failed due to EOF from client, just quit; there's no point in
***
*** 253,259 
  
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
!errmsg(errstr, port->user_name)));
/* doesn't return */
  }
  
--- 255,262 
  
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
!errmsg(errstr, port->user_name),
!errhint(hintstr)));
/* doesn't return */
  }
  

-- 
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] Proposed patch: make SQL interval-literal syntax work per spec

[Ron Mayer]

Tom Lane wrote:

Ron Mayer <[EMAIL PROTECTED]> writes:

'1Y1M'::interval  ... minute ... month

Hmmm.  I would say that the problem with that is not that it's
nonstandard but that it's ambiguous.  


Ah yes.


Our documentation...says..."or abbreviations".
...What if we just tweak the code to
reject ambiguous abbreviations?


Good idea.  I'll try that.


[ experiments a bit... ]  Another interesting point is that "mo",
which is a perfectly unique abbreviation, is rejected.  Seems like
the handling of abbreviations in this code could be improved.


It looks like rather than abbreviations being any shorter
form of a unit, it has an explicit list of abbreviations
it likes (deltatktbl) in the beginning of datetime.c that
forces "m" to "minute"?  So losing the ambiguous ones
should be very easy.

--
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] Proposed patch: make SQL interval-literal syntax work per spec

[Tom Lane]

Ron Mayer <[EMAIL PROTECTED]> writes:
> Oh.  I wasn't proposing 8601-only.  Just the one-character
> shorthands like '1Y1M'::interval that postgresql interprets
> as "1 year one minute".   No standard specifies anything close
> to that; and any similar standards ask to interpret that M as
> months instead of minutes.

Hmmm.  I would say that the problem with that is not that it's
nonstandard but that it's ambiguous.  Our documentation about the
interval type says:

  interval values can be written with the following syntax:

[EMAIL PROTECTED] quantity unit [quantity unit...] [direction]

  Where: quantity is a number (possibly signed); unit is microsecond,
  millisecond, second, minute, hour, day, week, month, year, decade,
  century, millennium, or abbreviations or plurals of these units;
  direction can be ago or empty. The at sign (@) is optional noise. The
  amounts of different units are implicitly added up with appropriate
  sign accounting. ago negates all the fields.

There isn't anything there that would suggest to a user that 'm' is
well-defined as a unit, much less that it specifically means "minute"
rather than one of the other options.  What if we just tweak the code to
reject ambiguous abbreviations?

[ experiments a bit... ]  Another interesting point is that "mo",
which is a perfectly unique abbreviation, is rejected.  Seems like
the handling of abbreviations in this code could be improved.

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] Proposed patch: make SQL interval-literal syntax work per spec

[Ron Mayer]

Tom Lane wrote:


The other problem is that the SQL spec clearly defines an interval
literal syntax, and it's not this ISO thing.  So even without backward
compatibility issues, 8601-only doesn't seem like it would fly.



Oh.  I wasn't proposing 8601-only.  Just the one-character
shorthands like '1Y1M'::interval that postgresql interprets
as "1 year one minute".   No standard specifies anything close
to that; and any similar standards ask to interpret that M as
months instead of minutes.

--
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] Proposed patch: make SQL interval-literal syntax work per spec

[Tom Lane]

Ron Mayer <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> I think *replacement* would be a hard sell, as that would tick off all
>> the existing users ;-).  Now it seems like being able to accept either

> I originally submitted a patch that supported both, and I think
> you suggested replacing on the grounds that the old one was
> never documented,

Yeah, but that was five years ago, and someone remedied the oversight
since then ...

The other problem is that the SQL spec clearly defines an interval
literal syntax, and it's not this ISO thing.  So even without backward
compatibility issues, 8601-only doesn't seem like it would fly.

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] [Review] pgbench duration option

[Tom Lane]

I wrote:
> Are people satisfied that the Windows part of the patch is okay?

I went ahead and applied this patch after some trivial stylistic fixes.
The buildfarm will soon tell us if the WIN32 part of the patch compiles,
but not whether it works --- would someone double-check the functioning
of the -T switch on Windows?

regards, tom lane

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

Re: [HACKERS] Proposed patch: make SQL interval-literal syntax work per spec

[Ron Mayer]

Tom Lane wrote:

Ron Mayer <[EMAIL PROTECTED]> writes:

Back a while ago (2003) there was some talk about replacing
some of the non-standard extensions with shorthand forms of
intervals with ISO 8601 intervals that have a similar but
not-the-same shorthand.


I think *replacement* would be a hard sell, as that would tick off all
the existing users ;-).  Now it seems like being able to accept either


I originally submitted a patch that supported both, and I think
you suggested replacing on the grounds that the old one was
never documented,

http://archives.postgresql.org/pgsql-patches/2003-09/msg00134.php
 "If we're going to support the real ISO spec, I'd suggest ripping
  out any not-quite-there variant.

http://archives.postgresql.org/pgsql-patches/2003-09/msg00121.php
 "I doubt anyone is using it, because it's completely undocumented."

On the other hand, the company I was at was indeed originally
using it, so I prefer that it stay in as well.   Perhaps if
there's a way to mark them as deprecated and post warnings in
the log file if they're used.   I think they should be
removed eventually in a few releases, because they're quite
confusing as they stand:

 IntervalISO Postgres
 8601shorthand
 -
 '1 year 1 minute'   'P1YT1M' '1Y1M'
 '1 year 1 month''P1Y1M'  N/A



the 8601 syntax or the existing syntaxes on input wouldn't be tough
at all, if you insist on the P prefix to distinguish; so that end of


ISO 8601 seems to me to require the P, so I think we would.


it should be easy enough.  On the output side, seems like a GUC variable
is the standard precedent here.  I'd still vote against overloading
DateStyle --- it does too much already --- but a separate variable for
interval style wouldn't bother me.  In fact, given that we are now
somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.


Great.  I'm bringing my patch up-to-date with CVS now
and adding the separate GUC.



--
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] Proposed patch: make SQL interval-literal syntax work per spec

[Tom Lane]

Ron Mayer <[EMAIL PROTECTED]> writes:
> Back a while ago (2003) there was some talk about replacing
> some of the non-standard extensions with shorthand forms of
> intervals with ISO 8601 intervals that have a similar but
> not-the-same shorthand.

I think *replacement* would be a hard sell, as that would tick off all
the existing users ;-).  Now it seems like being able to accept either
the 8601 syntax or the existing syntaxes on input wouldn't be tough
at all, if you insist on the P prefix to distinguish; so that end of
it should be easy enough.  On the output side, seems like a GUC variable
is the standard precedent here.  I'd still vote against overloading
DateStyle --- it does too much already --- but a separate variable for
interval style wouldn't bother me.  In fact, given that we are now
somewhat SQL-compliant on interval input, a GUC that selected
PG traditional, SQL-standard, or ISO 8601 interval output format seems
like it could be a good idea.

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] Commitfest patches mostly assigned ... status

[Andrew Chernow]

Josh Berkus wrote:

Josh is probably basing that on the long history of
discussion/revision cycles. 


Yep, and that *I* don't understand what the patch does, so I'm not going to 
turn a newbie reviewer loose on it.




Here is a quick overview, there are two parts to the patch:

1. event system
This allows one to register "PQregisterEventProc" a per-conn callback function 
with libpq that gets called when particular events occur.  Currently, the events 
tell you when a conn or result is created, reset, destroyed and copied.  It is 
generic enough to add more events in the future.


By receiving events about libpq objects, you can properly allocate and free 
userland memory (PQfinish, PQexec, PQclear, etc... trigger events) and associate 
it with the libpq object: thus PQsetInstanceData(conn...) or 
PQsetResultInstanceData(res).  This is basically adding members to the 
opaque PGconn or PGresult during runtime.  This "instance data" can be retreived 
via PQinstanceData and PQresultInstanceData.  To shine a different light on it, 
apps normally wrap a PGconn or PGresult within their own structures, but now you 
can wrap the app structures inside a PGconn or PGresult.


This may help, its the libpqtypes PGEventProc implementation.
http://libpqtypes.esilo.com/browse_source.html?file=events.c

Also check out the patches sgml docs if you get a chance.


2. result management
There are also four functions that provide more control over PGresult.  If you 
need to create a result from scratch, expands on the PQmakeEmptyPGResult idea.


PQcopyResult - copy a given source result, flags argument determines what 
portions of the result are copied.


PQsetResultAttrs - sets the attributes of the reuslt (its columns).

PQsetvalue - sets a tuple value in a result

PQresultAlloc - thin wrapper around the internal pqResultAlloc.  Uses the 
result's block allocater, which allows PQclear to properly free all memory 
assocaited with a PGresult.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] Proposed patch: make SQL interval-literal syntax work per spec

[Ron Mayer]

Tom Lane wrote:

"Kevin Grittner" <[EMAIL PROTECTED]> writes:
Tom Lane <[EMAIL PROTECTED]> wrote: 

I am not sure about some of the corner cases --- anyone want to see if
their understanding of the spec for  is different?
 

The patch seems to support extensions to the standard.


Right.  All of these were extensions that already existed in PG.


Back a while ago (2003) there was some talk about replacing
some of the non-standard extensions with shorthand forms of
intervals with ISO 8601 intervals that have a similar but
not-the-same shorthand.
 IntervalISO Postgres
 8601shorthand
 -
 '1 year 1 minute'   'P1YT1M' '1Y1M'
 '1 year 1 month''P1Y1M'  N/A
http://archives.postgresql.org/pgsql-patches/2003-09/msg00119.php

When I proposed we support the ISO-8601 standard shorthand,
Tom recommended we rip out the old shorthand at the same time:

http://archives.postgresql.org/pgsql-patches/2003-09/msg00134.php

I've been maintaining a patch that supports these ISO-8601
intervals for a client.  Back in 2003 I recall Peter said
he wanted to see SQL standard intervals first.  There were
also discussions about selecting the output format.  My old
patch made this depend on datestyle; but it seems Tom preferred
a separate GUC?

http://archives.postgresql.org/pgsql-patches/2003-12/msg00257.php

I see there's a TODO regarding ISO8601 intervals as well.

I have a version of this patch for 8.2; but would be happy
to bring it up-to-date if people want to re-consider it now.

--
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] Commitfest patches mostly assigned ... status

[Josh Berkus]

> Josh is probably basing that on the long history of
> discussion/revision cycles. 

Yep, and that *I* don't understand what the patch does, so I'm not going to 
turn a newbie reviewer loose on it.

-- 
--Josh

Josh Berkus
PostgreSQL
San Francisco

-- 
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] [Review] pgbench duration option

[Tom Lane]

Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> sys/time.h and unistd.h are #included just a few lines after that, but 
> within a #ifndef WIN32 block. I don't think the patch added any 
> codepaths where we'd need those header files on Windows, so I presume 
> that was just an oversight and those two extra #includes can be removed? 
> I don't have a Windows environment to test it myself.

Well, if we did need them the buildfarm would soon tell us, no?

> Also, should we be using pqsignal at all? It's not clear to me what it 
> is, to be honest, but there's a note in pqsignal.h that says "This 
> shouldn't be in libpq, but the monitor and some other things need it..."

Yeah, it has more portable semantics than just plain signal().

That note isn't offbase I suppose --- now that we have src/port/ it
would have been better to put it there.  But moving it now would be an
ABI break for libpq.so, and I don't think it's worth it.


Are people satisfied that the Windows part of the patch is okay?
I'm not able to review it meaningfully.  One thing that looks suspicious
is that handle_sig_alarm doesn't look like it's needed for Windows;
it'd be better if win32_timer_callback just did timer_exceeded = true
for itself.  (This might explain the bogus #include requirements?)

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] Commitfest patches mostly assigned ... status

[Merlin Moncure]

On Thu, Sep 11, 2008 at 5:06 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
>> The patch looks OK to me as it was the last time I looked at it; maybe
>> Tom has more comments, so I decided against just committing it.
>
> I haven't got round to looking at it in this fest.  If anyone else wants
> to look it over, feel free.  I think Josh is overrating the patch's
> review difficulty --- anyone who uses libpq a lot could review it,
> IMHO.  You certainly wouldn't need any backend-internals knowledge.
>

Josh is probably basing that on the long history of
discussion/revision cycles.  There is very little change from the
design that was hammered out except for the late breaking tweak of how
the passthru pointer works.  I think the subtext here is that we are
waiting on you to sign off on the patch (or not).

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] Commitfest patches mostly assigned ... status

[Tom Lane]

Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Merlin Moncure escribió:
>> Alvaro actually performed a review on libpq events.  We are waiting on
>> his feedback on our changes (based on his review) and the newly
>> submitted documentation.  I'll update the wiki accordingly.  I wasn't
>> sure if Alvaro was claiming reviewer status or not.  It may be ready
>> to go in as-is unless we draw any new objections.

> The patch looks OK to me as it was the last time I looked at it; maybe
> Tom has more comments, so I decided against just committing it.

I haven't got round to looking at it in this fest.  If anyone else wants
to look it over, feel free.  I think Josh is overrating the patch's
review difficulty --- anyone who uses libpq a lot could review it,
IMHO.  You certainly wouldn't need any backend-internals knowledge.

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] Parsing of pg_hba.conf and authentication inconsistencies

[Brendan Jurd]

Hi Magnus,

Josh has assigned your patch to me for an initial review.

First up I'd like to say that this is a really nice upgrade.
Shielding a running server from reloading a bogus conf file makes a
whole lot of sense.

On Sun, Aug 17, 2008 at 1:47 AM, Magnus Hagander <[EMAIL PROTECTED]> wrote:
>
> Attached is the patch I have so far. The only "extra" it adds over today
> is that it allows the use of "ident" authentication without explicitly
> specifying "sameuser" when you want that.
>

Nice.  I'd expect that custom ident maps are pretty rare, so this
seems like a good move.

> Other than that, it moves code around to do the parsing in the
> postmaster and the maching in the backend. This means that now if there
> is a syntax error in the file on a reload, we just keep the old file
> around still letting people log into the database. If there is a syntax
> error on server startup, it's FATAL of course, since we can't run
> without any kind of pg_hba.
>
> It also changes a couple of error cases to explicitly state that support
> for a certain auth method isn't compiled in, rather than just call it a
> syntax error.
>

The patch applied cleanly to HEAD, compiled fine on amd64 gentoo and
the features appeared to work as advertised.  I tried putting various
kinds of junk into my pg_hba.conf file and reloading/restarting the
postmaster to see how it would react.  As expected, on a reload I got
a parse error and a WARNING letting me know that the new configuration
was not loaded.  On a restart, I got the same parse error, and a FATAL
indicating that the postmaster couldn't start.

I also checked that it was safe to omit "sameuser" in an "ident"
record, and again that worked as expected.

The patch doesn't include any updates to documentation.  I humbly
suggest that the change to the ident map (making "sameuser" the
default) should be mentioned both in the Manual itself, and in the
sample conf file comments.  Here are a few sites that may be in want
of an update:

 * src/backend/libpq/pg_ident.conf.sample:33 - "Instead, use the
special map name "sameuser" in pg_hba.conf"

 * doc/src/sgml/client-auth.sgml:512 has a sample "ident sameuser" record

 * doc/src/sgml/client-auth.sgml:931 - "There is a predefined ident
map sameuser"

The section on Ident Authentication might need some broader rewording
to indicate that the map argument is now optional.

The new error messages are good, but I wonder if they could be
improved by using DETAIL segments.  The guidelines on error message
style say that the primary message should be terse and make a
reasonable effort to fit on one line.  For example, this:

LOG:  invalid IP address "a.b.c.d" in file
"/home/direvus/src/postgres/inst/data/pg_hba.conf" line 77: Name or
service not known

Could be rewritten as something like this:

LOG:  invalid IP address "a.b.c.d" in auth config: Name or service not known
DETAIL: In file "/home/direvus/src/postgres/inst/data/pg_hba.conf", line 77

That's all the feedback I have for the moment.  I hope you find my
comments helpful.

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: [HACKERS] Copy column storage parameters on CREATE TABLE LIKE/INHERITS

[Tom Lane]

ITAGAKI Takahiro <[EMAIL PROTECTED]> writes:
> Here is an updated version of the "Copy storage parameters" patch.
> http://archives.postgresql.org/pgsql-hackers/2008-07/msg01417.php

I looked this over and feel that it still needs work.  The
implementation seems appropriate for columns coming from LIKE clauses,
but there are two big issues for columns coming from inheritance parent
tables:

* The patch opens and acquires lock on the parent relations far too
early.  That is supposed to happen in DefineRelation, which among other
things contains the appropriate permissions check.  It would be possible
to use this patch to hold AccessShareLock for a long time on tables you
have no permissions for.

* There is no consideration for merging of similarly-named columns
coming from different parent tables.  The general approach taken in
DefineRelation is to throw an error if there are incompatible properties
in columns to be merged, and I think the same should happen for storage
properties.  What you actually would get, here, is that some random one
of the columns would "win".

In short, I think you need two implementations, one for LIKE and one
for INHERITS, and the appropriate place to tackle the latter case is in
DefineRelation (or even more specifically, MergeAttributes).

Also I concur with Stephen Frost's suggestion to add a couple of
cross-references to the documentation patches.

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] Synchronous Log Shipping Replication

[Markus Wanner]

Gregory Stark wrote:

Tom Lane <[EMAIL PROTECTED]> writes:


I'm not sure what to do if we need signals sent from processes that
aren't connected to shared memory; but maybe we need not cross that
bridge here.


Such as signals coming from the postmaster? Isn't that where most of them come
from though?


Uh.. no, such as signals *going to* the postmaster. That's where we 
already have such a multiplexer in place, but not the other way around IIRC.


Regards

Markus Wanner


--
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] Commitfest patches mostly assigned ... status

[Andrew Chernow]

Alvaro Herrera wrote:


Actually a minor gripe ... should PQsetvalue be PQsetValue? :-)



We were mimicing PQgetvalue, which uses a lowercase 'v'.  Is there a 
preference, none on this end.


--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] Commitfest patches mostly assigned ... status

[Alvaro Herrera]

Merlin Moncure escribió:
> On Thu, Sep 11, 2008 at 1:53 AM, Josh Berkus <[EMAIL PROTECTED]> wrote:
> > Some patches have not been assigned to reviewers for various reasons. The
> > following weren't assigned because they are complex and really need a
> > high-end hacker or committer to take them on:
> >
> > libpq events
> 
> Alvaro actually performed a review on libpq events.  We are waiting on
> his feedback on our changes (based on his review) and the newly
> submitted documentation.  I'll update the wiki accordingly.  I wasn't
> sure if Alvaro was claiming reviewer status or not.  It may be ready
> to go in as-is unless we draw any new objections.

The patch looks OK to me as it was the last time I looked at it; maybe
Tom has more comments, so I decided against just committing it.

I admit I haven't checked the docs.


Actually a minor gripe ... should PQsetvalue be PQsetValue? :-)

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

Re: [HACKERS] Commitfest patches mostly assigned ... status

[Merlin Moncure]

On Thu, Sep 11, 2008 at 1:53 AM, Josh Berkus <[EMAIL PROTECTED]> wrote:
> Some patches have not been assigned to reviewers for various reasons. The
> following weren't assigned because they are complex and really need a
> high-end hacker or committer to take them on:
>
> libpq events

Alvaro actually performed a review on libpq events.  We are waiting on
his feedback on our changes (based on his review) and the newly
submitted documentation.  I'll update the wiki accordingly.  I wasn't
sure if Alvaro was claiming reviewer status or not.  It may be ready
to go in as-is unless we draw any new objections.

Anybody who wants to look should ping Alvaro and/or Tom.

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] no XLOG during COPY?

[Andrew Dunstan]

Heikki Linnakangas wrote:

Andrew Dunstan wrote:
Back in February, Tom said here: 
http://archives.postgresql.org/pgsql-hackers/2008-02/msg00963.php :



That defeats a couple of optimizations that
Simon put in recently.  The one for no XLOG during COPY is not too
hard to see how to re-enable, but I'm not sure what else there was.


Could someone please point me at where this optimization was 
committed? I'm having trouble locating it.


http://archives.postgresql.org/pgsql-committers/2007-01/msg00296.php



Great, thanks (and also to Guillaume).

It looks to me like the simple way around this issue would be to provide 
an option to have pg_restore emit:

   begin; truncate foo; copy foo ... commit;

The truncate will be trivial as there won't be any data or indexes at 
that stage anyway.


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] no XLOG during COPY?

[Heikki Linnakangas]

Andrew Dunstan wrote:
Back in February, Tom said here: 
http://archives.postgresql.org/pgsql-hackers/2008-02/msg00963.php :



That defeats a couple of optimizations that
Simon put in recently.  The one for no XLOG during COPY is not too
hard to see how to re-enable, but I'm not sure what else there was.


Could someone please point me at where this optimization was committed? 
I'm having trouble locating it.


http://archives.postgresql.org/pgsql-committers/2007-01/msg00296.php

--
  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] no XLOG during COPY?

[Guillaume Smet]

On Thu, Sep 11, 2008 at 9:01 PM, Andrew Dunstan <[EMAIL PROTECTED]> wrote:
> Could someone please point me at where this optimization was committed? I'm
> having trouble locating it.

I think it's this one:
http://archives.postgresql.org/pgsql-committers/2007-01/msg00296.php

-- 
Guillaume

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

[HACKERS] no XLOG during COPY?

[Andrew Dunstan]

Back in February, Tom said here: 
http://archives.postgresql.org/pgsql-hackers/2008-02/msg00963.php :



That defeats a couple of optimizations that
Simon put in recently.  The one for no XLOG during COPY is not too
hard to see how to re-enable, but I'm not sure what else there was.

  


Could someone please point me at where this optimization was committed? 
I'm having trouble locating it.


Thanks

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] NDirectFileRead and Write

[Tom Lane]

ITAGAKI Takahiro <[EMAIL PROTECTED]> writes:
> Here is a patch to user NDirectFileRead/Write counters to get I/O counts
> in BufFile module. We can see the counters when log_statement_stats is on.

Couple thoughts here:

* Let's rename them BufFileReadCount and BufFileWriteCount to reflect
their actual purpose.

* If the extern's are moved to buffile.h, I think the definitions of the
variables themselves should move to buffile.c.  However, that would
imply including buffile.h in bufmgr.c which is a bit ugly from a
modularity standpoint.  In any case I agree that the current arrangement
with execdebug.h declaring variables defined in bufmgr.c is just weird.

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] Commitfest patches mostly assigned ... status

[Tom Lane]

Josh Berkus <[EMAIL PROTECTED]> writes:
> Also, note that the following patches need performance testing on a 
> variety of platforms.  Everyone should help with this.
> GSoC Improved Hash Indexing
> posix fadvises
> operator restrictivity function for text search
> CLUSTER using sort instead of index scan

The last two of those are not code-complete so I'm not sure that
performance testing is all that meaningful for them.  But by all
means, test the first two ...

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] [PATCHES] to_date() validation

[Brendan Jurd]

On Fri, Sep 12, 2008 at 3:34 AM, Tom Lane <[EMAIL PROTECTED]> wrote:
> Applied with minimal modifications.  I changed a couple of error
> messages that didn't seem to meet the style guidelines,

Great, thanks Tom.  And thanks again to Alex and Martijn for helping
me refine the patch.

> and I moved all
> of the to_timestamp tests into horology.sql, rather than having
> essentially duplicate tests in timestamp.sql and timestamptz.sql.
>

Nice.  That will make maintaining/extending the tests easier in future.

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: [HACKERS] Transaction Snapshots and Hot Standby

[Chris Browne]

[EMAIL PROTECTED] (Heikki Linnakangas) writes:
> Simon Riggs wrote:
>> Taking snapshots from primary has a few disadvantages
>>
>>  ...
>>   * snapshots on primary prevent row removal (but this was also an
>> advantage of this technique!)
>
> That makes it an awful solution for high availability. A backend hung
> in transaction-in-progress state in the slave will prevent row removal
> on the master. Isolating the master from queries done performed in the
> slave is exactly the reason why people use hot standby. And running
> long reporting queries in the standby is again a very typical use case.

I agree that this is a demerit to this approach.

Whether or not, on balance, it makes it an 'awful solution for high
availability' is much more in the eye of the beholder, and NOT obvious
on the face of it.
-- 
let name="cbbrowne" and tld="linuxdatabases.info" in name ^ "@" ^ tld;;
http://linuxfinances.info/info/sgml.html
Question: How many surrealists does it take to change a light bulb?

Answer: Two, one to hold the giraffe, and the other to fill the bathtub
with brightly colored machine tools.

-- 
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] [PATCHES] to_date() validation

[Tom Lane]

"Brendan Jurd" <[EMAIL PROTECTED]> writes:
> I've attached a new version of the patch (version 3), as well as an
> incremental patch to show the differences between versions 2 and 3.

Applied with minimal modifications.  I changed a couple of error
messages that didn't seem to meet the style guidelines, and I moved all
of the to_timestamp tests into horology.sql, rather than having
essentially duplicate tests in timestamp.sql and timestamptz.sql.

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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Gregory Stark wrote:

Heikki Linnakangas <[EMAIL PROTECTED]> writes:


BTW, we haven't talked about how to acquire a snapshot in the slave. You'll
somehow need to know which transactions have not yet committed, but will in the
future. 


I'm not sure why you need to know which ones will commit in the future.


Hmm, I phrased that badly. We need to know which transactions *might* 
commit in the future, IOW, are still in progress. Because we want to 
mark those as in-progress in the snapshots that are taken in the slave. 
Otherwise, when they do commit, they will suddenly become visible in the 
snapshots that didn't know that they were in progress.


--
  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] What is d2mdir?

[Devrim GÜNDÜZ]

On Tue, 2008-09-02 at 12:47 -0400, Alvaro Herrera wrote:
> Unknown SDATA: [mdash ]
> at /usr/share/sgml/docbook/utils-0.6.14/helpers//docbook2man-spec.pl
> line 1241,  line 11975.
> 
> Please see here:
> 
> http://archives.postgresql.org/message-id/20080821130203.GN4169%
> 40alvh.no-ip.org

FWIW, Fedora folks fixed it in Fedora-9, and updated to latest version
as of Fedora 10.

Regards,
-- 
Devrim GÜNDÜZ, RHCE
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
   http://www.gunduz.org


signature.asc
Description: This is a digitally signed message part

Re: [HACKERS] Synchronous Log Shipping Replication

[Gregory Stark]

Tom Lane <[EMAIL PROTECTED]> writes:

> I'm not sure what to do if we need signals sent from processes that
> aren't connected to shared memory; but maybe we need not cross that
> bridge here.

Such as signals coming from the postmaster? Isn't that where most of them come
from though?

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL 
training!

-- 
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] Transaction Snapshots and Hot Standby

[Gregory Stark]

Heikki Linnakangas <[EMAIL PROTECTED]> writes:

> BTW, we haven't talked about how to acquire a snapshot in the slave. You'll
> somehow need to know which transactions have not yet committed, but will in 
> the
> future. 

I'm not sure why you need to know which ones will commit in the future. ISTM
you need the same information you normally have which is just which ones have
committed as of the point of WAL replay you're at.

However it does occur to me that if you know in advance that a transaction
will abort in the future you could skip applying its WAL records when you see
them. I'm not sure it's worth adding such an optimization though and it might
get weird around vacuum.

> In the master, we keep track of in-progress transaction in the ProcArray, so
> I suppose we'll need to do the same in the slave. Very similar to prepared
> transactions, actually. I believe the Abort records, which are not actually
> needed for normal operation, become critical here. The slave will need to
> put an entry to ProcArray for any new XLogRecord.xl_xid it sees in the WAL,
> and remove the entry at a Commit and Abort record. And clear them all at a
> shutdown record.

That's how I envisioned it when the topic came up, but another solution was
also bandied about -- I'm not sure who originally proposed it. The alternative
method is to have the master periodically insert a data structure describing a
snapshot in the WAL. The slave then keeps a copy of the last snapshot it saw
in the WAL and any new query which starts up uses that.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS support!

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

Re: [HACKERS] Potential Join Performance Issue

[Tom Lane]

"Lawrence, Ramon" <[EMAIL PROTECTED]> writes:
> To keep the changes simple, the update simply calls
> ExecChooseHashTableSize() in create_hashjoin_plan() to re-calculate the
> expected number of batches.  This is more efficient and results in less
> code changes than modifying the HashPath struct to store the number of
> batches and updating that variable when costing (as cost_hashjoin() will
> be called many times during costing).

I was intending to do it the other way, actually.  An extra field in
HashPath hardly costs anything.  The other reason for it is that there
are other possible uses for knowing whether a hash will be multi-batch.
(For example, if we were prepared to tell the executor that it *must*
keep the hash to one batch, we could assume that the sort order of the
left input is preserved.  I haven't looked into the risks/benefits of
that too much, but it's been in the back of the mind for a long time.)

> An ideal solution would detect at execution time if the inner relation
> remained in memory (one batch) and decide to disable/enable the
> physical-tlist optimization on the outer relation accordingly.  At this
> time, we are uncertain if this would be desirable or possible.

That seems pretty infeasible really.  Aside from changing plan node
output tuple types on-the-fly, it would mean renumbering Vars in the
join node to reference the outer relation's new output columns.  The
overhead of supporting that would be paid across-the-board in the
executor whether or not anyone got any real benefit from it.

I'd be more inclined to deal with the issue by trying to establish a
"safety margin" in the estimate of whether the hash will go multi-batch.
IOW we should disuse_physical_tlist if the hash is estimated to be close
to but still within one batch.

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] Synchronous Log Shipping Replication

[Markus Wanner]

Hi,

Tom Lane wrote:

No, that's not what I had in mind at all, just the ability to deliver
one of a specified set of event notifications --- ie, get around the
fact that Unix only gives us two user-definable signal types.


Ah, okay. And I already thought you'd like imessages :-(


For signals sent from other backends, it'd be sufficient to put a
bitmask field into PGPROC entries, which the sender could OR bits into
before sending the one "real" signal event (either SIGUSR1 or SIGUSR2).


That might work for expanding the number of available signals, yes.

Regards

Markus Wanner

--
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] [Review] pgbench duration option

[Heikki Linnakangas]

ITAGAKI Takahiro wrote:

***
*** 29,36 
--- 29,40 
  #include "postgres_fe.h"
  
  #include "libpq-fe.h"

+ #include "pqsignal.h"
  
  #include 

+ #include 
+ #include 
+ #include 
  
  #ifdef WIN32

  #undef FD_SETSIZE


sys/time.h and unistd.h are #included just a few lines after that, but 
within a #ifndef WIN32 block. I don't think the patch added any 
codepaths where we'd need those header files on Windows, so I presume 
that was just an oversight and those two extra #includes can be removed? 
I don't have a Windows environment to test it myself.


Also, should we be using pqsignal at all? It's not clear to me what it 
is, to be honest, but there's a note in pqsignal.h that says "This 
shouldn't be in libpq, but the monitor and some other things need it..."


--
  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] Synchronous Log Shipping Replication

[Tom Lane]

Markus Wanner <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Sooner or later we shall have to bite the bullet and set up a
>> multiplexing system to transmit multiple event types to backends with
>> just one signal.  We already did it for signals to the postmaster.

> Agreed. However, it's non-trivial if you want reliable queues (i.e. no 
> message skipped, as with signals) for varying message sizes.

No, that's not what I had in mind at all, just the ability to deliver
one of a specified set of event notifications --- ie, get around the
fact that Unix only gives us two user-definable signal types.

For signals sent from other backends, it'd be sufficient to put a
bitmask field into PGPROC entries, which the sender could OR bits into
before sending the one "real" signal event (either SIGUSR1 or SIGUSR2).

I'm not sure what to do if we need signals sent from processes that
aren't connected to shared memory; but maybe we need not cross that
bridge here.

(Also, I gather that the Windows implementation could already support
a bunch more signal types without much trouble.)

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

[Alvaro Herrera]

Jorgen Austvik - Sun Norway wrote:

> The attached patch makes pg_regress write converted files to  
> /sql and /expected, which is one way to make it read  
> and write to the same directory. Tested on Solaris x86 with pgsql "make  
> check" and standalone.

Okay, so this patch does change it in a way that it still works, but
what else do you need to be able to run the test from another directory?
I tried to run the test from another directory with this patch
installed, and found that it didn't work because it's replacing
@abs_builddir@ in the input files improperly (to the current path; it
should be using the output dir path, I think)

So maybe this is a step in the right direction, but ISTM you need a
slightly larger patch for it to be actually useful.

If I am not making sense, then maybe I am not understanding what you
mean by running it standalone.  In that case, please explain.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

Re: [HACKERS] Synchronous Log Shipping Replication

[Heikki Linnakangas]

Fujii Masao wrote:

I think that this case would often happen. So, we should establish a certain
solution or procedure to the case where TLI of the master doesn't match
TLI of the slave. If we only allow the case where TLI of both servers is the
same, the configuration after failover always needs to get the base backup
on the new master. It's unacceptable for many users. But, I think that it's
the role of admin or external tools to copy history files to the slave from
the master.


Hmm. There's more problems than the TLI with that. For the original 
master to catch up by replaying WAL from the new slave, without 
restoring from a full backup, the original master must not write to disk 
*any* WAL that hasn't made it to the slave yet. That is certainly not 
true for asynchronous replication, but it also throws off the idea of 
flushing the WAL concurrently to the local disk and to the slave in 
synchronous mode.


I agree that having to get a new base backup to get the old master catch 
up with the new master sucks, so I hope someone sees a way around 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: [HACKERS] Synchronous Log Shipping Replication

[Fujii Masao]

On Thu, Sep 11, 2008 at 3:17 AM, Simon Riggs <[EMAIL PROTECTED]> wrote:
>
> On Wed, 2008-09-10 at 17:57 +0900, Fujii Masao wrote:
>
>> My sequence covers several cases :
>>
>> * There is no missing WAL file.
>> * There is a lot of missing WAL file.
>
> This is the likely case for any medium+ sized database.

I'm sorry, but I could not understand what you mean.

>
>> * There are missing history files. Failover always generates the gap
>> of
>>history file because TLI is incremented when archive recovery is
>> completed.
>
> Yes, but failover doesn't happen while we are configuring replication,
> it can only happen after we have configured replication. It would be
> theoretically possible to take a copy from one server and then try to
> synchronise with a 3rd copy of the same server, but that seems perverse
> and bug prone. So I advise that we only allow replication when the
> timeline of the standby matches the timeline of the master, having it as
> an explicit check.

Umm... my explanation seems to have been unclear:(
Here is the case which I assume.

1) Configuration of replication, i.e. the master and the slave work fine.
2) The master fails down, then failover happens. When the slave becomes
 the master, TLI is incremented, and new history file is generated.
3) In order to catch up with the new master, the server which was the master
from the first needs missing history file. At this time, it's
because there is
the gap of TLI in between two servers.

I think that this case would often happen. So, we should establish a certain
solution or procedure to the case where TLI of the master doesn't match
TLI of the slave. If we only allow the case where TLI of both servers is the
same, the configuration after failover always needs to get the base backup
on the new master. It's unacceptable for many users. But, I think that it's
the role of admin or external tools to copy history files to the slave from
the master.

>> In your design, does not initial setup block the master?
>> Does your design cover above-mentioned case?
>
> The way I described it does not block the master. It does defer the
> point at which we can start using synchronous replication, so perhaps
> that is your objection. I think it is acceptable: good food takes time
> to cook.

Yes. I understood your design.

> IMHO it will be confusing to be transferring both old and new data at
> the same time from master to slave. We will have two different processes
> sending and two different processes receiving. You'll need to work
> through about four times as many failure modes, all of which will need
> testing. Diagnosing problems in it via the log hurts my head just
> thinking about it. ISTM that will severely impact the initial robustness
> of the software for this feature. Perhaps in time it is the right way.

In my procedure, old WAL files are copyed by admin using scp, rsync
or other external tool. So, I don't think that my procedure makes a
problem more difficult. Since there are many setup cases, we should
not leave all procedures to postgres, I think.

> Anyway, feels like we're getting close to some good designs. There isn't
> much difference between what we're discussing here.

Yes. Thank you for your great ideas.

-- 
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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Simon Riggs wrote:

So part of the handshake between
primary and standby must be "what is your recentxmin?". The primary will
then use the lower/earliest of the two.


Even then, the master might already have vacuumed away tuples that are 
visible to an already running transaction in the slave, before the slave 
connects. Presumably the master doesn't wait for the slave to connect 
before starting to accept new connections.


As you mentioned, the options there are to defer applying WAL, or cancel 
queries. I think both options need the same ability to detect when 
you're about to remove a tuple that's still visible to some snapshot, 
just the action is different. We should probably provide a GUC to 
control which you want.


I don't see any practical way of telling whether a tuple removal will
affect a snapshot or not. Each removed row would need to be checked
against each standby snapshot. Even if those were available, it would be
too costly. 


How about using the same method as we use in HeapTupleSatisfiesVacuum? 
Before replaying a vacuum record, look at the xmax of the tuple 
(assuming it committed). If it's < slave's OldestXmin, it can be 
removed. Otherwise not. Like HeapTupleSatisfiesVacuum, it's 
conservative, but doesn't require any extra bookkeeping.


And vice versa: if we implement the more precise book-keeping, with all 
snapshots in shared memory or something, we might as well use it in 
HeapTupleSatisfiesVacuum. That has been discussed before, but it's a 
separate project.



It was also suggested we might take the removed rows and put them in a
side table, but that makes me think of the earlier ideas for HOT and so
I've steered clear of that.


Yeah, that's non-trivial. Basically a whole new, different 
implementation of MVCC, but without changing any on-disk formats.


BTW, we haven't talked about how to acquire a snapshot in the slave. 
You'll somehow need to know which transactions have not yet committed, 
but will in the future. In the master, we keep track of in-progress 
transaction in the ProcArray, so I suppose we'll need to do the same in 
the slave. Very similar to prepared transactions, actually. I believe 
the Abort records, which are not actually needed for normal operation, 
become critical here. The slave will need to put an entry to ProcArray 
for any new XLogRecord.xl_xid it sees in the WAL, and remove the entry 
at a Commit and Abort record. And clear them all at a shutdown record.


--
  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] Move src/tools/backend/ to wiki

[Alvaro Herrera]

Peter Eisentraut wrote:
> Alvaro Herrera wrote:
>> So I went ahead and moved its mention to a separate question, where it
>> has a lot more visibility.  (I also added an URL to anoncvs, where
>> people can see it more readily.)
>
> Perhaps the low visibility also has to do with the fact that  
> documentation is hidden under "tools".

Well, with the way it was before it was even more hidden.  Now at least
it has its own question, and has working links and all.

However I wonder how much value there really is in the developer's FAQ,
considering that some answers seem rather poor.  For example the
answer on ereport() was wrong, and nobody ever pointed it out.  The
answer on palloc/pfree is very incomplete too.

My question is, is this resource actually useful for somebody?

> Once  we start applying the argument that things should be moved to
> the wiki  based on getting more people to work on it, we might as well
> move the  source code to the wiki altogether. ;-)

Hey, now that's a clever idea!

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

Re: [HACKERS] Transaction Snapshots and Hot Standby

[Simon Riggs]

Thanks for the detailed thinking. At least one very good new idea here,
some debate on other points.


On Thu, 2008-09-11 at 09:24 +0300, Heikki Linnakangas wrote:

> And still we can't escape the scenario that the slave receives a WAL 
> record that vacuums away a tuple that's still visible according to a 
> snapshot used in the slave. Even with the proposed scheme, this can happen:
> 
> 1. Slave receives a snapshot from master
> 2. A long-running transaction begins on the slave, using that snapshot
> 3. Network connection is lost
> 4. Master hits a timeout, and decides to discard the snapshot it sent to 
> the slave
> 5. A tuple visible to the snapshot is vacuumed
> 6. Network connection is re-established
> 7. Slave receives the vacuum WAL record, even though the long-running 
> transaction still needs the tuple.

Interesting point. (4) is a problem, though not for the reason you
suggest. If we were to stop and start master, that would be sufficient
to discard the snapshot that the standby is using and so cause problems.
So the standby *must* tell the master the recentxmin it is using, as you
suggest later, so good thinking. So part of the handshake between
primary and standby must be "what is your recentxmin?". The primary will
then use the lower/earliest of the two.

> I like the idea of acquiring snapshots locally in the slave much more.

Me too. We just need to know how, if at all.
 
> As you mentioned, the options there are to defer applying WAL, or cancel 
> queries. I think both options need the same ability to detect when 
> you're about to remove a tuple that's still visible to some snapshot, 
> just the action is different. We should probably provide a GUC to 
> control which you want.

I don't see any practical way of telling whether a tuple removal will
affect a snapshot or not. Each removed row would need to be checked
against each standby snapshot. Even if those were available, it would be
too costly. And even if we can do that, ISTM that neither option is
acceptable: if we cancel queries then touching a frequently updated
table is nearly impossible, or if we delay applying WAL then the standby
could fall behind, impairing its ability for use in HA. (If there was a
way, yes, we should have a parameter for it).

It was also suggested we might take the removed rows and put them in a
side table, but that makes me think of the earlier ideas for HOT and so
I've steered clear of that.

You might detect blocks that have had tuples removed from them *after* a
query started by either
* keeping a hash table of changed blocks - it would be a very big data
structure and hard to keep clean
* adding an additional "last cleaned LSN" onto every data block
* keeping an extra LSN on the bufhdr for each of the shared_buffers,
plus keeping a hash table of blocks that have been cleaned and then
paged out
Once detected, your only option is to cancel the query.

ISTM if we want to try to avoid making recentxmin same on both primary
and standby then the only viable options are the 3 on the original post.

> However, if we still to provide the behavior that "as long as the 
> network connection works, the master will not remove tuples still needed 
> in the slave" as an option, a lot simpler implementation is to 
> periodically send the slave's oldest xmin to master. Master can take 
> that into account when calculating its own oldest xmin. That requires a 
> lot less communication than the proposed scheme to send snapshots back 
> and forth. A softer version of that is also possible, where the master 
> obeys the slave's oldest xmin, but only up to a point.

I like this very much. Much simpler implementation and no need for a
delay in granting snapshots. I'll go for this as the default
implementation. Thanks for the idea.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Csaba Nagy wrote:

and that means in fact that if you have
continuously overlapping small transactions, the "blocking horizon"
could be even blocked forever, as there'll always be a query running,
and the new queries will always have the snapshot of the currently
running ones because WAL recovery is stalled... 


Hmm, no I don't think the WAL recovery can become completely stalled. To 
completely stop progressing, we'd need to take a new snapshot that 
includes transaction X, and at the same time be blocked on a vacuum 
record that vacuums a tuple that's visible to transaction X. I don't 
think that can happen, because for such a scenario to arise, in the 
corresponding point in time in the master, there would've been a 
scenario where the vacuum would've removed a tuple that would have been 
 visible to a newly starting transaction. Which can't happen. I think..


--
  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] Transaction Snapshots and Hot Standby

[Dimitri Fontaine]

Le jeudi 11 septembre 2008, Csaba Nagy a écrit :
> Well now that I think I understand what Heikki meant, I also think the
> problem is that there's no choice at all to advance, because the new
> queries will simply have the same snapshot as currently running ones as
> long as WAL reply is blocked... further blocking the WAL reply. When
> saying this I suppose that the snapshot is in fact based on the last
> recovered XID, and not on any slave-local XID. In that case once WAL
> recovery is blocked, the snapshot is stalled too, further blocking WAL
> recovery, and so on...

Well, it may be possible to instruct the WAL replay daemon to stop being 
polite sometimes: when a given max_lag_delay is reached, it could take locks 
and as soon as it obtains them, replay all remaining WAL.
The max_lag_delay would be a GUC allowing to set the threshold between 
continuing the replay and running queries.

There could some other nice ideas without inventing yet another GUC, but this 
one was eaiser to think about for me ;)
-- 
dim


signature.asc
Description: This is a digitally signed message part.

Re: [HACKERS] Transaction Snapshots and Hot Standby

[Csaba Nagy]

On Thu, 2008-09-11 at 15:33 +0200, Dimitri Fontaine wrote:
> What would forbid the slave to choose to replay all currently lagging WALs 
> each time it's given the choice to advance a little?

Well now that I think I understand what Heikki meant, I also think the
problem is that there's no choice at all to advance, because the new
queries will simply have the same snapshot as currently running ones as
long as WAL reply is blocked... further blocking the WAL reply. When
saying this I suppose that the snapshot is in fact based on the last
recovered XID, and not on any slave-local XID. In that case once WAL
recovery is blocked, the snapshot is stalled too, further blocking WAL
recovery, and so on...

Cheers,
Csaba.



-- 
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] Transaction Snapshots and Hot Standby

[Dimitri Fontaine]

Le jeudi 11 septembre 2008, Heikki Linnakangas a écrit :
> Well, yes, but you can fall behind indefinitely that way. Imagine that
> each transaction on the slave lasts, say 10 minutes, with a new
> transaction starting every 5 minutes. On the master, there's a table
> that's being vacuumed (or HOT-updated) frequently, say after each
> transaction for simplicity. What can happen is that every transaction
> that finishes on the slave will only let the WAL replay advance by one
> XID before blocking on the snapshot of the next slave transaction. The
> WAL replay will advance at a rate of 0.2 TPM, while the master is
> generating 1.0 TPM.

What would forbid the slave to choose to replay all currently lagging WALs 
each time it's given the choice to advance a little?
-- 
dim


signature.asc
Description: This is a digitally signed message part.

Re: [HACKERS] Transaction Snapshots and Hot Standby

[Csaba Nagy]

On Thu, 2008-09-11 at 16:19 +0300, Heikki Linnakangas wrote:
> Well, yes, but you can fall behind indefinitely that way. Imagine that 
> each transaction on the slave lasts, say 10 minutes, with a new 
> transaction starting every 5 minutes. On the master, there's a table 
> that's being vacuumed (or HOT-updated) frequently, say after each 
> transaction for simplicity. What can happen is that every transaction 
> that finishes on the slave will only let the WAL replay advance by one 
> XID before blocking on the snapshot of the next slave transaction. The 
> WAL replay will advance at a rate of 0.2 TPM, while the master is 
> generating 1.0 TPM.

Aha, now I see where I was mistaken... I thought in terms of time and
not transaction IDs. So the time distance between the slave transactions
does not matter at all, only the distance in recovered XIDs matter for
the "blocking horizon"... and if the WAL recovery is blocked, the
"blocking horizon" is stalled as well, so the next transaction on the
slave will in fact require the same "blocking horizon" as all currently
running ones. Now I got it... and that means in fact that if you have
continuously overlapping small transactions, the "blocking horizon"
could be even blocked forever, as there'll always be a query running,
and the new queries will always have the snapshot of the currently
running ones because WAL recovery is stalled... or at least that's what
I understand from the whole thing...

Cheers,
Csaba.



-- 
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] Transaction Snapshots and Hot Standby

[Robert Haas]

> I'd imagine that even if applying the WAL on the slave is blocked, it's
> still streamed from the master to the slave, and in case of failover the
> slave will fast-forward before starting up as the new master. Of course, if
> it has fallen 3 days behind because of a giant reporting query, it can take
> a while to replay all the WAL that has accumulated.

Yes, and meanwhile, any other queries that are running on that box are
seeing three-day old data as well.

In an ideal world, it would be nice if the slave could keep tuples
around even after they are dead and vacuumed on the master.  Pushing
Xmin from the slave to the master creates the possibility of bloating
the master due to activity on the slave, which is not going to improve
reliability.  OTOH, not pushing Xmin leads to several pathological
query behaviors on the slave: cancelled queries, inconsistent reads,
and falling way behind on WAL application.  Either way, it seems to me
a massive and uncomfortable violation of the POLA.

If it were possible for tuples that had been vacuumed on the master to
stick around on the slave for as long as the slave still needed them,
then you'd have the best of both worlds, but I have a feeling
someone's going to say that that's just about impossible to implement.
 Against that, all I can say is that neither of the behaviors
described thus far sounds very appealing as a feature, though I'm
certain there are some people who, with sufficient jiggering, could
make effective use of them.

...Robert

-- 
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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Csaba Nagy wrote:

On Thu, 2008-09-11 at 15:42 +0300, Heikki Linnakangas wrote:
One problem with this, BTW, is that if there's a continuous stream of 
medium-length transaction in the slave, each new snapshot taken will 
prevent progress in the WAL replay, so the WAL replay will advance in 
"baby steps", and can fall behind indefinitely.


Why would it fall behind indefinitely ? It only should fall behind to
the "blocking horizon", which should be the start of the longest
currently running transaction... which should be continually advancing
and not too far in the past if there are only medium length transactions
involved.


Well, yes, but you can fall behind indefinitely that way. Imagine that 
each transaction on the slave lasts, say 10 minutes, with a new 
transaction starting every 5 minutes. On the master, there's a table 
that's being vacuumed (or HOT-updated) frequently, say after each 
transaction for simplicity. What can happen is that every transaction 
that finishes on the slave will only let the WAL replay advance by one 
XID before blocking on the snapshot of the next slave transaction. The 
WAL replay will advance at a rate of 0.2 TPM, while the master is 
generating 1.0 TPM.


--
  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] Transaction Snapshots and Hot Standby

[Jochem van Dieten]

On Thu, Sep 11, 2008 at 2:07 AM, Simon Riggs wrote:
> Transaction snapshots is probably the most difficult problem for Hot
> Standby to resolve.

>  * remotely from primary node
>  * locally on the standby node
>
> If we derive a snapshot locally, then we will end up with a situation
> where the xmin of the local snapshot precedes the xmin of the primary
> node. When this occurs it will then be possible for WAL records to
> arrive on the standby that request removal of rows that a transaction
> might wish to see. Preventing that situation can be done by either
> deferring WAL apply or by cancelling queries.

Which operations can request row removal? Isn't that just specific
operations that have their own 'is this save to remove' calculations
anyway (i.e. vacuum and HOT prune)?

What I am thinking about is a design where the primary node were to
regularly push an OldestXMin into the WAL, the WAL apply process on
the standby nodes pushes it into shared memory and the backends keep
an OldestMasterXMin in shared memory. The standby nodes then regularly
pushes back the oldest OldestMasterXMin from all backends to the
master. Vacuum and HOT prune could then base their calculations on an
OldestXMin that is not the OldestXMin of the master itself, but of the
master and the standby nodes. That way removal of records that are
still visible on one of the standby nodes is prevented on the master
instead of worked around on the standby nodes.

The obvious downside would be bloat on the master (which could get out
of hand if a slave is a few days behind due to a long report), but I
think in terms of visibility and consistency this would work. Or am I
completely misunderstanding the problem?

Jochem

-- 
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] Synchronous Log Shipping Replication

[Markus Wanner]

Hi,

Tom Lane wrote:

Sooner or later we shall have to bite the bullet and set up a
multiplexing system to transmit multiple event types to backends with
just one signal.  We already did it for signals to the postmaster.


Agreed. However, it's non-trivial if you want reliable queues (i.e. no 
message skipped, as with signals) for varying message sizes. My 
imessages stuff is certainly not perfect, yet. But it works to some 
extent and provides exactly that functionality.


However, I'd be happy to work on improving it, if other projects start 
using it as well. Anybody else interested? Use cases within Postgres 
itself as of now?


Regards

Markus Wanner


--
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] Synchronous Log Shipping Replication

[Markus Wanner]

Hi,

Fujii Masao wrote:

Umm... backends have already used SIGUSR1. PostgresMain() sets up a signal
handler for SIGUSR1 as follows.


Uh.. right. Thanks for pointing that out. Maybe just use SIGPIPE for now?


Yes, since WAL sender waits on select(), it's convenient to use signal
for the notification *from backends to WAL sender*, I think too.


..and I'd say you better use the same for WAL sender to backend 
communication, just for the sake of simplicity (and thus maintainability).


Regards

Markus Wanner

--
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] Synchronous Log Shipping Replication

[Tom Lane]

"Fujii Masao" <[EMAIL PROTECTED]> writes:
> Which signal should WAL sender send to backends?

Sooner or later we shall have to bite the bullet and set up a
multiplexing system to transmit multiple event types to backends with
just one signal.  We already did it for signals to the postmaster.

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] Transaction Snapshots and Hot Standby

[Csaba Nagy]

On Thu, 2008-09-11 at 15:42 +0300, Heikki Linnakangas wrote:
> One problem with this, BTW, is that if there's a continuous stream of 
> medium-length transaction in the slave, each new snapshot taken will 
> prevent progress in the WAL replay, so the WAL replay will advance in 
> "baby steps", and can fall behind indefinitely.

Why would it fall behind indefinitely ? It only should fall behind to
the "blocking horizon", which should be the start of the longest
currently running transaction... which should be continually advancing
and not too far in the past if there are only medium length transactions
involved. Isn't normal WAL recovery also doing baby-steps, one WAL
record a time ? ;-)

Cheers,
Csaba.



-- 
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] Synchronous Log Shipping Replication

[Fujii Masao]

On Wed, Sep 10, 2008 at 11:13 PM, Markus Wanner <[EMAIL PROTECTED]> wrote:
>> Which signal should we use for the notification to the backend from
>> WAL sender? The notable signals are already used.
>
> I'm using SIGUSR1, see src/backend/storage/ipc/imsg.c from Postgres-R, line
> 232. That isn't is use for backends or the postmaster, AFAIK.

Umm... backends have already used SIGUSR1. PostgresMain() sets up a signal
handler for SIGUSR1 as follows.

 pqsignal(SIGUSR1, CatchupInterruptHandler);

Which signal should WAL sender send to backends?

>> Or, since a backend don't need to wait on select() unlike WAL sender,
>> ISTM that it's not so inconvenient to use a semaphore for that
>> notification.
>
> They probably could, but not the WAL sender.

Yes, since WAL sender waits on select(), it's convenient to use signal
for the notification *from backends to WAL sender*, I think too.

Best 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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Csaba Nagy wrote:

On Thu, 2008-09-11 at 15:23 +0300, Heikki Linnakangas wrote:
I'd imagine that even if applying the WAL on the slave is blocked, it's 
still streamed from the master to the slave, and in case of failover the 
slave will fast-forward before starting up as the new master.


Which begs the question: what happens with a query which is running on
the slave in the moment when the slave switches from recovery mode and
starts up ? Should the running queries be canceled if they are blocking
applying of WAL, to allow start-up, or let them finish ?


Depends on application, I'd say. I guess we'll need both, like the smart 
and fast shutdown modes.


--
  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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Gregory Stark wrote:

b) vacuum on the server which cleans up a tuple the slave has in scope has to
   block WAL reply on the slave (which I suppose defeats the purpose of having
   a live standby for users concerned more with fail-over latency).


One problem with this, BTW, is that if there's a continuous stream of 
medium-length transaction in the slave, each new snapshot taken will 
prevent progress in the WAL replay, so the WAL replay will advance in 
"baby steps", and can fall behind indefinitely. As soon as there's a 
moment that there's no active snapshot, it can catch up, but if the 
slave is seriously busy, that might never happen.


Nevertheless, I think it's a much nicer approach.

--
  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] Transaction Snapshots and Hot Standby

[Csaba Nagy]

On Thu, 2008-09-11 at 15:23 +0300, Heikki Linnakangas wrote:
> I'd imagine that even if applying the WAL on the slave is blocked, it's 
> still streamed from the master to the slave, and in case of failover the 
> slave will fast-forward before starting up as the new master.

Which begs the question: what happens with a query which is running on
the slave in the moment when the slave switches from recovery mode and
starts up ? Should the running queries be canceled if they are blocking
applying of WAL, to allow start-up, or let them finish ?

Cheers,
Csaba.



-- 
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] Transaction Snapshots and Hot Standby

[Heikki Linnakangas]

Merlin Moncure wrote:

There is nothing stopping you from setting up two (or more) slave
servers, with one designated as failover that doens't serve queries,
right?


I'd imagine that even if applying the WAL on the slave is blocked, it's 
still streamed from the master to the slave, and in case of failover the 
slave will fast-forward before starting up as the new master. Of course, 
if it has fallen 3 days behind because of a giant reporting query, it 
can take a while to replay all the WAL that has accumulated.


--
  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] [PATCH] Cleanup of GUC units code

[Robert Haas]

> temp_buffers is actually special-cased in the code because
>
>   /*
>* We show the GUC var until local buffers have been initialized, and
>* NLocBuffer afterwards.
>*/
>
> It is not clear to me right now why that is a good idea.  But it is only
> this one paramter.

OK, well that's not so bad then, although it would be nice to make it
consistent.

> The actual logic that SHOW uses in the general case is to reformat the value
> with the largest unit that allows for an integer value to be presented.  So
> this is neither your nor Greg's idea, but I think it gives useful behavior
> in practice.

Yes, that's a totally sensible choice as well.  What do you think of
the idea of always requiring an explicit unit, either by deprecating
blocks as a unit altogether, or pushing users to specify "blocks" in
that case?  It seemed to me from reading your previous response that
you thought this would make it more possible to be more flexible about
how MB, GB, etc. are specified, although I'm not exactly sure what the
relationship is.

...Robert

-- 
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] Transaction Snapshots and Hot Standby

[Merlin Moncure]

On Thu, Sep 11, 2008 at 7:18 AM, Gregory Stark <[EMAIL PROTECTED]> wrote:
> a) transactions with live snapshots on the slave prevent the master from being
>   able to vacuum tuples (which defeats the purpose of having a live standby
>   server for some users).
>
> or
>
> b) vacuum on the server which cleans up a tuple the slave has in scope has to
>   block WAL reply on the slave (which I suppose defeats the purpose of having
>   a live standby for users concerned more with fail-over latency).

There is nothing stopping you from setting up two (or more) slave
servers, with one designated as failover that doens't serve queries,
right?  Option b seems pretty reasonable to me, although I'd prefer to
block wal replay vs canceling queries...although it might be nice to
manually be able to force wal replay 'with query cancel' via a
checkpoint.

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] Interesting glitch in autovacuum

[Tom Lane]

Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Actually, 8.2 initializes it to InvalidTransactionId too, so it doesn't
> seem like there's a problem there either.  Since the problem stems from
> using it as initializer for the Min() calculation, there's no actual bug
> on 8.2 either.  The bug only appeared on 8.3 when the initializer was
> changed.  And given that there's no HOT in 8.2, then there's no danger
> of misusing it in page pruning either.

I concur that 8.2 has no problem except in vac_update_datfrozenxid,
but I think it is an actual bug there.  If newFrozenXid starts out as 
InvalidTransactionId, it'll stay that way because InvalidTransactionId
sorts as older than anything else.  The result will be that the routine
fails to advance datfrozenxid, which leads to exactly the type of
autovacuum misbehavior that I saw in HEAD.  (Actually there's another
problem in an assert-enabled build: it'll fail at the
Assert(TransactionIdIsNormal(newFrozenXid)) ...)

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] New FSM patch

[Zdenek Kotala]

Another question:

Does we need random_bool to spread workload? It seems to me a useless, because 
it also invokes one backend to use more pages instead of using one which is 
already in buffer cache. I think that it should generate a lot of extra i/o. Do 
not forget WAL full page write for firstime modified page.


Zdenek


--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
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] Move src/tools/backend/ to wiki

[Peter Eisentraut]

Alvaro Herrera wrote:

So I went ahead and moved its mention to a separate question, where it
has a lot more visibility.  (I also added an URL to anoncvs, where
people can see it more readily.)


Perhaps the low visibility also has to do with the fact that 
documentation is hidden under "tools".



What I'm wondering right now is what's the value of having this stuff
in the source code at all.  Why don't we move it to the Wiki and remove
it from the repo?  Would anybody object to that idea?  Having it on CVS
means that nobody is able to improve the text, which is kind of sad
because it's been neglected for the last 3 years.


I think it is documentation and should therefore be kept in CVS.  Once 
we start applying the argument that things should be moved to the wiki 
based on getting more people to work on it, we might as well move the 
source code to the wiki altogether. ;-)


--
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] Cleanup of GUC units code

[Tom Lane]

Peter Eisentraut <[EMAIL PROTECTED]> writes:
> temp_buffers is actually special-cased in the code because

> /*
>  * We show the GUC var until local buffers have been initialized, and
>  * NLocBuffer afterwards.
>  */

> It is not clear to me right now why that is a good idea.

I think the reason for it is that you can change the setting within a
particular session, but only up until temp buffers have been initialized
(ie, when you first touch a temp table).  If we just made it act like
other GUCs then SHOW would be lying to you about the effective value
if you changed it after that point.

An easy workaround would be to make the variable PGC_BACKEND, but that
seems to me to lose useful flexibility.  Or we could invent a context
category specifically for this type of behavior, but is it worth that?

The narrowest fix would be to just teach the show hook to format its
result properly.  I seem to recall having looked at that and been
annoyed by the amount of copy-and-paste required.

[ thinks for a bit... ]  Or maybe we should get rid of the show hook
and instead put in an assign hook that prevents changes after the
initialization event.  The problem is that you can't throw error
while reading, eg, a new config file.  So while this might be the most
user-friendly approach, I think it would take some API change for assign
hooks.

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] New FSM patch

[Heikki Linnakangas]

Zdenek Kotala wrote:
I have question about FSM structure on the page. If I look into the code 
it seems to me that tree structure is "degenerated" on the right side. 
It is probably space optimization because complete tree needs BLCKSZ/2 
space on the page and rest is empty.


Is my assumption correct? If yes maybe extra note in fsm_internal.h 
about it could be helpful.


Yes, that's right. I'll add a note on that if it helps.

--
  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] New FSM patch

[Zdenek Kotala]

I have question about FSM structure on the page. If I look into the code it 
seems to me that tree structure is "degenerated" on the right side. It is 
probably space optimization because complete tree needs BLCKSZ/2 space on the 
page and rest is empty.


Is my assumption correct? If yes maybe extra note in fsm_internal.h about it 
could be helpful.



Zdenek




--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


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

Re: [HACKERS] Postgresql coding conventions

[Tom Lane]

Gregory Stark <[EMAIL PROTECTED]> writes:
> Abbas <[EMAIL PROTECTED]> writes:
>> While writing code or reviewing a path are we supposed to consider the
>> camel cased names correct or the under-score separated names correct?

> Some parts of the code use the two to distinguish between functions local to
> that module and functions that are part of the public api. In those cases
> functions with capitals are part of the public api of the module and lower
> case functions are internal functions or utility functions. Except for the
> modules where it's the reverse...

Right --- there are enough different naming styles in the codebase now
that it seems pretty much hopeless to expect it ever to be just one style.
I think the most we can hope for now is "try to make your patch
consistent with nearby code".  Which is definitely a point worth
considering for reviewers --- just don't expect that there's one true style.

> And actually looking around I can't find any good examples of this
> where there aren't exceptions. I don't like the idea of a massive
> cleanup patch for this but if someone's doing major surgery on a
> module it could be worth fixing up names in that module to be
> consistent at the same time.

If it's a total rewrite anyway, then of course you could use whatever
names you like.  But for incremental changes I would vote against
changing names that aren't directly being touched by the patch.  What
that would mainly accomplish is to cause fits for the poor sods who have
to back-patch bug fixes.  I still routinely curse our decision to alter
pg_indent's comment-wrapping rules around 8.1, because it means that no
back-patch ever applies cleanly across the 8.1/8.2 boundary.

A contrary point here is that if you are changing a function's API,
or the meaning of a field, etc, it's often a good idea to intentionally
rename it; that guarantees that you won't miss fixing any references
to 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] [PATCH] Cleanup of GUC units code

[Peter Eisentraut]

Robert Haas wrote:

A good start might be to always OUTPUT memory parameters using the
same base unit.

SHOW gives output that matches what you input.


Not for me it doesn't.

portal=# show temp_buffers;
 temp_buffers
--
 1024
(1 row)

portal=# set temp_buffers = '16MB';
SET
portal=# show temp_buffers;
 temp_buffers
--
 2048
(1 row)


temp_buffers is actually special-cased in the code because

   /*
* We show the GUC var until local buffers have been initialized, and
* NLocBuffer afterwards.
*/

It is not clear to me right now why that is a good idea.  But it is only 
this one paramter.


The actual logic that SHOW uses in the general case is to reformat the 
value with the largest unit that allows for an integer value to be 
presented.  So this is neither your nor Greg's idea, but I think it 
gives useful behavior in practice.



--
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] Transaction Snapshots and Hot Standby

[Gregory Stark]

Simon Riggs <[EMAIL PROTECTED]> writes:

> On Thu, 2008-09-11 at 11:11 +0100, Richard Huxton wrote:
>
>> I have to say I agree with Heikki here. Blocking the master based on
>> what the slave is doing seems to make host standby less useful than warm.
>
> I agree also, that why I flagged it up for discussion.

So as far as I can see there are basically two option here. Either

a) transactions with live snapshots on the slave prevent the master from being
   able to vacuum tuples (which defeats the purpose of having a live standby
   server for some users).

or

b) vacuum on the server which cleans up a tuple the slave has in scope has to
   block WAL reply on the slave (which I suppose defeats the purpose of having
   a live standby for users concerned more with fail-over latency).

Is there any middle ground or brilliant ways to get the best of both worlds?

If not it seems to me the latter is preferable since at least the consequence
of having a long-running query on the slave occurs on the same machine running
the query. And the work-around -- killing the long-running query -- requires
taking action on the same machine as the consequences. Also, when you take
action it fixes the problem immediately as WAL reply can recommence which
seems like a better deal than a bloated database.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's 24x7 Postgres 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] Interesting glitch in autovacuum

[Tom Lane]

"Pavan Deolasee" <[EMAIL PROTECTED]> writes:
> On Wed, Sep 10, 2008 at 9:22 PM, Tom Lane <[EMAIL PROTECTED]> wrote:
>> On reflection I'm not even sure that this is strictly an autovacuum
>> bug.  It can be cast more generically as "RecentGlobalXmin getting
>> used without ever having been set", and it sure looks to me like the
>> HOT patch may have introduced a few risks of that sort.

> ISTM that HOT may be safe here because even if RecentGlobalXmin is not
> set and has the boot time value of FirstNormalTransactionId, the
> heap_page_prune_opt() would just return without doing any real work.

Until the XID counter advances past the 2billion mark.  Then the
uninitialized RecentGlobalXmin would be "in the future" and would
result in mistaken decisions *to* prune, not to not prune.

In short this is a risk-of-data-loss bug.  Once the patch is in
I think we ought to start thinking about a set of update releases...

regards, tom lane

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

Re: [HACKERS] Postgresql coding conventions

[Gregory Stark]

Abbas <[EMAIL PROTECTED]> writes:

> While writing code or reviewing a path are we supposed to consider the
> camel cased names correct or the under-score separated names correct?

Some parts of the code use the two to distinguish between functions local to
that module and functions that are part of the public api. In those cases
functions with capitals are part of the public api of the module and lower
case functions are internal functions or utility functions. Except for the
modules where it's the reverse...

And actually looking around I can't find any good examples of this where there
aren't exceptions. I don't like the idea of a massive cleanup patch for this
but if someone's doing major surgery on a module it could be worth fixing up
names in that module to be consistent at the same time.


-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS support!

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

Re: [HACKERS] [Review] pgbench duration option

[Magnus Hagander]

Heikki Linnakangas wrote:
> ITAGAKI Takahiro wrote:
>> Here is a revised version of the pgbench duration patch.
> 
> Looking at the Win32 timer implementation, it's a bit different from the
> one we have in src/backend/port/win32/timer.c. The one in timer.c uses a
> separate thread and WaitForSingleObjectEx() to wait, while your
> implementation uses CreateTimerQueue() and CreateTimerQueueTimer().
> Yours seems simpler, so I wonder why the timer.c is different?

Probably because it was written back when we supported NT4, and the
CreateTimerQueue() stuff requires Windows 2000 to work.


> It's not too bad as it is in the patch, but it would be nice to put the
> setitimer() implementation into src/port, and use the same code in the
> backend as well.

I haven't looked at the patch ;-), but the implementation in
backend/port is tied into the signal emulation layer that's also in
backend/port, so I think doing such a move will require moving a lot
more than just the timer code...

//Magnus

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

Re: [HACKERS] Transaction Snapshots and Hot Standby

[Simon Riggs]

On Thu, 2008-09-11 at 11:11 +0100, Richard Huxton wrote:

> I have to say I agree with Heikki here. Blocking the master based on
> what the slave is doing seems to make host standby less useful than warm.

I agree also, that why I flagged it up for discussion.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and 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] Postgresql coding conventions

[Andrew Dunstan]

Abbas wrote:

Hi,
I have noticed two different coding conventions being followed in
postgres code base.

See e.g. function names in syslogger.c

static void set_next_rotation_time(void);
static void sigHupHandler(SIGNAL_ARGS);

and variable names in the same file

int bytes_in_logbuffer = 0;
char*currentLogDir;

Chapter 46 of the documentation does not say much about variable or
function naming.

While writing code or reviewing a path are we supposed to consider the
camel cased names correct or the under-score separated names correct?


  


I don't think we have a standard. If there is to be one I'll cast as 
many votes as possible for the use of underscores. 
readingWithoutSpacesReallySucks.


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] [Review] pgbench duration option

[Heikki Linnakangas]

ITAGAKI Takahiro wrote:

Here is a revised version of the pgbench duration patch.


Looking at the Win32 timer implementation, it's a bit different from the 
one we have in src/backend/port/win32/timer.c. The one in timer.c uses a 
separate thread and WaitForSingleObjectEx() to wait, while your 
implementation uses CreateTimerQueue() and CreateTimerQueueTimer(). 
Yours seems simpler, so I wonder why the timer.c is different?


It's not too bad as it is in the patch, but it would be nice to put the 
setitimer() implementation into src/port, and use the same code in the 
backend as well.


--
  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] Transaction Snapshots and Hot Standby

[Richard Huxton]

Heikki Linnakangas wrote:
> Simon Riggs wrote:
>> Taking snapshots from primary has a few disadvantages
>>
>>  ...
>>   * snapshots on primary prevent row removal (but this was also an
>> advantage of this technique!)
> 
> That makes it an awful solution for high availability. A backend hung in
> transaction-in-progress state in the slave will prevent row removal on
> the master. Isolating the master from queries done performed in the
> slave is exactly the reason why people use hot standby. And running long
> reporting queries in the standby is again a very typical use case.

I have to say I agree with Heikki here. Blocking the master based on
what the slave is doing seems to make host standby less useful than warm.

> I like the idea of acquiring snapshots locally in the slave much more.

It's the option that I can see people (well, me) understanding the
easiest. All the others sound like ways to get things wrong. As for
inconsistent query-results - that way madness lies. How on earth will
anyone be able to diagnose or report bugs when they occur?

> As you mentioned, the options there are to defer applying WAL, or cancel
> queries. I think both options need the same ability to detect when
> you're about to remove a tuple that's still visible to some snapshot,
> just the action is different. We should probably provide a GUC to
> control which you want.

I think there's only one value here: "hot standby wal delay time before
cancelling query". Might be a shorter name.

-- 
  Richard Huxton
  Archonet Ltd

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

Re: [HACKERS] Postgresql coding conventions

[Heikki Linnakangas]

Abbas wrote:

I have noticed two different coding conventions being followed in
postgres code base.

See e.g. function names in syslogger.c

static void set_next_rotation_time(void);
static void sigHupHandler(SIGNAL_ARGS);

and variable names in the same file

int bytes_in_logbuffer = 0;
char*currentLogDir;

Chapter 46 of the documentation does not say much about variable or
function naming.

While writing code or reviewing a path are we supposed to consider the
camel cased names correct or the under-score separated names correct?


Both styles are in use in different parts of the source tree, mainly for 
historical reasons. The rule of thumb is to see what style is used in 
the surrounding code, and follow 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: [HACKERS] Transaction Snapshots and Hot Standby

[Simon Riggs]

On Thu, 2008-09-11 at 09:24 +0300, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > Taking snapshots from primary has a few disadvantages
> > 
> >  ...
> >   * snapshots on primary prevent row removal (but this was also an
> > advantage of this technique!)
> 
> That makes it an awful solution for high availability. 

Please be careful about making such statements. People might think you
were opposing the whole idea of Hot Standby, rather than giving an
opinion about one suggestion out of many implementation proposals.

Looks like you've got some good additional suggestions later in the
post. I'll reply later to those, so thanks for that.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


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

[HACKERS] Postgresql coding conventions

[Abbas]

Hi,
I have noticed two different coding conventions being followed in
postgres code base.

See e.g. function names in syslogger.c

static void set_next_rotation_time(void);
static void sigHupHandler(SIGNAL_ARGS);

and variable names in the same file

int bytes_in_logbuffer = 0;
char*currentLogDir;

Chapter 46 of the documentation does not say much about variable or
function naming.

While writing code or reviewing a path are we supposed to consider the
camel cased names correct or the under-score separated names correct?

Regards
Abbas
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] Transaction Snapshots and Hot Standby

[Hannu Krosing]

On Thu, 2008-09-11 at 09:24 +0300, Heikki Linnakangas wrote:

> I like the idea of acquiring snapshots locally in the slave much more. 
> As you mentioned, the options there are to defer applying WAL, or cancel 
> queries. 

More exotic ways to defer applying WAL include using some smart
filesystems to get per-backend data snapshots, using either
copy-of-write overlay filesystems and filesystem or disk level
snapshots.

Al least the disk level snapshots exist in SAN-s with aim of easing
backups, though I'm not sure if it is effective for use hot standby
intended use.

Using any of those needs detecting and bypassing shared buffers if they
hold "too new" data pages and reading these pages directly from disk
snapshot.

> I think both options need the same ability to detect when 
> you're about to remove a tuple that's still visible to some snapshot, 
> just the action is different. We should probably provide a GUC to 
> control which you want.

We probably need to have two LSN's per page to make maximal use of our
MVCC in Hot Standby situation, so we can distinguish addition to a page,
which implies no data loss from row removal which does. Currently only
Vacuum and Hot pruning can cause row removal.

> However, if we still to provide the behavior that "as long as the 
> network connection works, the master will not remove tuples still needed 
> in the slave" as an option, a lot simpler implementation is to 
> periodically send the slave's oldest xmin to master. Master can take 
> that into account when calculating its own oldest xmin. That requires a 
> lot less communication than the proposed scheme to send snapshots back 
> and forth. A softer version of that is also possible, where the master 
> obeys the slave's oldest xmin, but only up to a point.

That point could be statement_timeout or (currently missing)
transaction_timeout

Also, decision to advance xmin should probably be sent to slave as well,
even though it is not something that is needed in local WAL logs.

--
Hannu



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

Re: [PATCHES] [HACKERS] TODO item: Implement Boyer-Moore searching (First time hacker)

[Heikki Linnakangas]

David Rowley wrote:

The thing that surprised me was that string_to_array didn't perform as well.
I expected single character searches to perform a little better. I can't
think why it would be slower now.


Yes, that's strange. I tried to reproduce that here, with a CVS snapshot 
before the patch, and after. With quick testing with psql and \timing 
and the same query you had in that spreadsheet, I couldn't see that kind 
of performance degradation. Oprofile suggests that, on the contrary, 
slightly less time is spent in text_position_next() after the patch, and 
slightly more in text_position_setup(). Together they account for ~10% 
of CPU time in both tests, so a small difference there would be 
insignificant anyway in that test case.


I think we're fine.

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