date:20100101

On Fri, 2010-01-01 at 12:53 +0200, Heikki Linnakangas wrote:
 Simon Riggs wrote:
  Attached is the patch I intend to commit, barring objections.
  
  This patch extends SIGINT to allow cancellation of transactions while
  idle in both HS and normal mode.
 
 How does AbortTransactionAndAnySubtransactions() differ from
 AbortOutOfAnyTransaction()? Having followed the discussions on this
 patch I think I know the answer: I'm guessing that
 AbortTransactionAndAnySubtransactions() leaves the backend in aborted
 state, while AbortOutOfAnyTransaction() leaves it in idle state. It
 would be good to point that out more clearly in the comment above
 AbortTransactionAndAnySubtransactions().

OK

 I agree with Joachim's comments upthread that we should have a different
 function for forcefully aborting the whole transaction, and leave the
 current pg_cancel_backend() behavior alone. 

That would require multiple behaviours. Joachim already proposed
multiplexing SIGINT and Tom disagreed, on the simultaneous thread Hot
Standby introduced problem with query cancel behaviour. I agree that
there seems little point in multiplexing both signals.

So the only way to have multiple cancel behaviours is to do this
cancellation via SIGUSR1 options and have additional functions to
request them.

Which amounts to rejecting this patch, since *this* patch changes the
behaviour of SIGINT for all senders, which is what I understood people
desired, i.e. not just a change for Hot Standby. I assumed Joachim did
not mean to veto his own patch, but I'm not sure what you think here?
(I don't mind either way).

 In any case, the
 documentation of pg_cancel_backend() or the new function needs to
 explain what exactly it does.

...The patch changes the docs for pg_cancel_backend().

 I believe people liked the idea of giving a different ERROR message when
 a transaction is canceled because of conflict with recovery. It would
 also be good to give a different error message when an idle transaction
 is canceled using query cancel. Otherwise you don't know what hit you,
 especially if you're not paying attention to NOTICEs.

I did like that idea when I heard it, but it seemed to have a few
problems on implementation.

Currently we say 

ERROR:  current transaction is aborted, commands ignored until end of
transaction block

and we repeat this every time a new statement is sent other than COMMIT
or ROLLBACK.

We could either endlessly repeat this

ERROR:  current transaction is aborted because of conflict with
recovery, commands ignored until end of transaction block

or just say it once and then revert to the normal message. Neither
seemed very useful.

I'm also not sure why we would want to single out Hot Standby to
generate the reason because of conflict with recovery when no other
ERROR source would generate such a reason.

  It also changes the standard message
  reported on an idle transaction in aborted state to 'IDLE in
  transaction (aborted)', so that once aborted we keep the message even if
  the user tries to issue further statements other than ROLLBACK or
  COMMIT.
 
 That's nice.

-- 
 Simon Riggs   www.2ndQuadrant.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] krb_server_keyfile setting doesn't work on Windows

On Thu, Dec 31, 2009 at 00:57, Magnus Hagander mag...@hagander.net wrote:
2009/12/31 Hiroshi Inoue in...@tpf.co.jp:
Magnus Hagander wrote:

2009/12/30 Hiroshi Inoue in...@tpf.co.jp:

Hi,

As far as I tested, the krb_server_keyfile setting
in postgres.conf doesn't work on Windows.

Because gssapi32.dll(krb5_32.dll) seems to call
getenv(KRB5_KTNAME) in msvcr71, postgres.exe
should call putenv(KRB5_KTNAME=...) in msvcr71.
The attached patch fixes the problem in my test
case.

Isn't the main backend linked with msvcr71.dll anyway?

What main backend directly links is msvcr80 or msvcr90
according to the msvc build environment.

Arrgh. My bad, I thought msvcr71 was vs2005. Now that you put it like
this, I know it's vs2003.

Then the
regular putenv should put it in th eenv of msvcr71.dll, and the stuff
that's wrapped through src/port/win32env.c will put it in the regular
msvcr file.

I wonder if you're possibly being hit with the bug I patched the other
day, but didn't backpatch.
(http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=f8bcd7220b1166f7c037ceaf0a53958cbc6a7630).

Can you see if that fix solves your problem as well? (Either directly
or by testing HEAD)

I'm testing using the current cvs.

If not, the fix should still go in win32env.c, not directly in auth.c

I don't object to it. Possibly we would have to add msvcr80 or
msvcr90 as well in the future.

To be safe, yes, we should have that. Do you want to work on such a
complete solution, or should I look at it?

Mail to you seem not to be properly delivered atm :-( Hopefully you
can read this through the list.

I've applied a patch that should fix this problem, by always updating
*all* available msvcrt libraries. Please check that it solves your
problem as well.

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

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

Re: [HACKERS] Cancelling idle in transaction state

2010-01-01 Thread Robert Haas


On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w

We could either endlessly repeat this

ERROR:  current transaction is aborted because of conflict with
recovery, commands ignored until end of transaction block


+1 for this option.


I'm also not sure why we would want to single out Hot Standby to
generate the reason because of conflict with recovery when no other
ERROR source would generate such a reason.


Well, most times when the transaction is aborted, it's because you did  
something wrong.  Or at least, the failure is associated with some  
particular statement.


If we have other events that can asynchronously roll back a  
transaction, I would think they would deserve similar handling.  Off  
the top of my head, I'm not sure if there are any such cases.


...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] Cancelling idle in transaction state

2010-01-01 Thread Heikki Linnakangas

Simon Riggs wrote:
 On Fri, 2010-01-01 at 12:53 +0200, Heikki Linnakangas wrote:
 I agree with Joachim's comments upthread that we should have a different
 function for forcefully aborting the whole transaction, and leave the
 current pg_cancel_backend() behavior alone. 
 
 That would require multiple behaviours. Joachim already proposed
 multiplexing SIGINT and Tom disagreed, on the simultaneous thread Hot
 Standby introduced problem with query cancel behaviour. I agree that
 there seems little point in multiplexing both signals.
 
 So the only way to have multiple cancel behaviours is to do this
 cancellation via SIGUSR1 options and have additional functions to
 request them.
 
 Which amounts to rejecting this patch, since *this* patch changes the
 behaviour of SIGINT for all senders, which is what I understood people
 desired, i.e. not just a change for Hot Standby. I assumed Joachim did
 not mean to veto his own patch, but I'm not sure what you think here?
 (I don't mind either way).

I don't know, I don't feel strongly about this. Is there really no other
way?

 In any case, the
 documentation of pg_cancel_backend() or the new function needs to
 explain what exactly it does.
 
 ...The patch changes the docs for pg_cancel_backend().

I don't think that's enough. Cancel a backend's current query or abort
an idle transaction leaves a lot of questions. When does it abort the
transaction? The whole top-level transaction or just the current
subtransaction?

 I believe people liked the idea of giving a different ERROR message when
 a transaction is canceled because of conflict with recovery. It would
 also be good to give a different error message when an idle transaction
 is canceled using query cancel. Otherwise you don't know what hit you,
 especially if you're not paying attention to NOTICEs.
 
 I did like that idea when I heard it, but it seemed to have a few
 problems on implementation.
 
 Currently we say 
 
 ERROR:  current transaction is aborted, commands ignored until end of
 transaction block
 
 and we repeat this every time a new statement is sent other than COMMIT
 or ROLLBACK.
 
 We could either endlessly repeat this
 
 ERROR:  current transaction is aborted because of conflict with
 recovery, commands ignored until end of transaction block
 
 or just say it once and then revert to the normal message. Neither
 seemed very useful.

Seems useful to me, so that you know why your transaction was cancelled.
It's rather weird to see no ERRORs in the previous steps, and suddenly
you see that the transaction is aborted. And none your savepoints exist
anymore either.

 I'm also not sure why we would want to single out Hot Standby to
 generate the reason because of conflict with recovery when no other
 ERROR source would generate such a reason.

Because you don't get any other ERROR for it. If your transaction is
aborted because of e.g a unique key violation, you get the ERROR about
unique key violation first, and only the subsequent commands produce the
current transaction is aborted message. With hot standby conflicts,
the first and only symptom the client sees is that the subsequent
commands fail with current transaction is aborted, so it would be nice
to know why.

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

[HACKERS] Change to config.pl processing in the msvc build environment

I'd like to apply the attached patch to the msvc build environment,
which changes how config.pl is handled. (For the unenlightened, this
is the replacement stuff we have for autoconf)

Today, there is one config.pl. We ship it with a couple of defaults
(which points to a very old installation of mine, really, and probably
shouldn't have included those paths in the first place), and you are
supposed to edit this config.pl file in-place to change which 3rd
party libs etc you have.

This is pretty convenient for end-users building from source. Of which
we don't have many. And it's annoying for doing any development work
on it, because it shows up in your git diff or whatever, and you
can't ignore the file or you won't get new updates out.

I therefor propose that we rename this file to config.pl.default,
and change the scripts to first load config.pl.default, and then load
config.pl if it exists. config.pl then lives completely outside the
source tree (should be in .cvsignore) and won't show up in any diffs
or anything.

It changes the format of the config.pl file (not the default one), so
that the user can now specify just one or two options, and doesn't hav
eto respecify all. So now you can have just:
$config-{zlib}='c:\zlib';

(or similar).

*If* you put in th place the old config.pl file, it will override
whatever is in the default file. Thus, I don't believe this would have
any effect on the buildfarm or the oneclick-installer build
environments.


Comments? Objectsions? Holes in the reasoning? :-)

(patch excludes the rename of config.pl to config.pl.default, for readability)

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


msvc_config.patch
Description: Binary data

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

Re: [HACKERS] Cancelling idle in transaction state

2010-01-01 Thread Gurjeet Singh

On Fri, Jan 1, 2010 at 8:38 PM, Robert Haas robertmh...@gmail.com wrote:

 On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w

  We could either endlessly repeat this

 ERROR:  current transaction is aborted because of conflict with
 recovery, commands ignored until end of transaction block


 +1 for this option.


  I'm also not sure why we would want to single out Hot Standby to
 generate the reason because of conflict with recovery when no other
 ERROR source would generate such a reason.


 Well, most times when the transaction is aborted, it's because you did
 something wrong.  Or at least, the failure is associated with some
 particular statement.

 If we have other events that can asynchronously roll back a transaction, I
 would think they would deserve similar handling.  Off the top of my head,
 I'm not sure if there are any such cases.


Why not do the finger pointing (to HS) in the DETAIL field of the ERROR, and
let the error message remain the same.

Best regards,
-- 
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.enterprisedb.com

singh.gurj...@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet

Mail sent from my BlackLaptop device

Re: [HACKERS] Cancelling idle in transaction state

On Fri, 2010-01-01 at 07:08 -0800, Robert Haas wrote:
 On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w
  We could either endlessly repeat this
 
  ERROR:  current transaction is aborted because of conflict with
  recovery, commands ignored until end of transaction block
 
 +1 for this option.
 
  I'm also not sure why we would want to single out Hot Standby to
  generate the reason because of conflict with recovery when no other
  ERROR source would generate such a reason.
 
 Well, most times when the transaction is aborted, it's because you did  
 something wrong.  Or at least, the failure is associated with some  
 particular statement.
 
 If we have other events that can asynchronously roll back a  
 transaction, I would think they would deserve similar handling.  Off  
 the top of my head, I'm not sure if there are any such cases.

Serialization failures, deadlocks, timeouts, SIGINT, out of memory
errors etc..

-- 
 Simon Riggs   www.2ndQuadrant.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] about some parameters

Jaime Casanova jcasa...@systemguards.com.ec writes:
 Every time i explain what is the fsync parameter for, the next thing i
 always say is: never turn it off, and now that we have
 synchronous_commit there is no good reason for turn fsync off... so
 why are we still let it be in the postgresql.conf where it's so
 vulnerable to a misguided dba?

It's a never turn it off *in production* kind of setting.  That
doesn't make it useless --- in development or debug situations it
might be just fine.

The larger picture here is that just making sure fsync is on doesn't
mean squat as far as the safety of your data goes :-(.  You also need
to be sure that every hardware/software layer below Postgres is honoring
fsync; which is a much harder thing to be sure of.  In fact, one of the
most reliable end-to-end tests I can think of for that is to see whether
turning off fsync creates a major speed change.  So we would not really
accomplish much by removing or hiding the parameter, and we would take
away a simple but important test procedure.

 another parameter that is interesting is seq_page_cost, AFAIUI all the
 other cost parameters (in the planner section of course) are relative
 to this one. so what is the logic in allow changing it?

Please read the discussions that went on when we added that parameter.

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] Cancelling idle in transaction state

2010-01-01 Thread Robert Haas


On Jan 1, 2010, at 8:30 AM, Simon Riggs si...@2ndquadrant.com wrote:

On Fri, 2010-01-01 at 07:08 -0800, Robert Haas wrote:

On Jan 1, 2010, at 6:48 AM, Simon Riggs si...@2ndquadrant.com w

We could either endlessly repeat this

ERROR:  current transaction is aborted because of conflict with
recovery, commands ignored until end of transaction block


+1 for this option.


I'm also not sure why we would want to single out Hot Standby to
generate the reason because of conflict with recovery when no  
other

ERROR source would generate such a reason.


Well, most times when the transaction is aborted, it's because you  
did

something wrong.  Or at least, the failure is associated with some
particular statement.

If we have other events that can asynchronously roll back a
transaction, I would think they would deserve similar handling.  Off
the top of my head, I'm not sure if there are any such cases.


Serialization failures, deadlocks, timeouts, SIGINT, out of memory
errors etc..


Hmm. I don't think I can get a serialization failure, deadlock, or out  
of memory error while my session is idle. An idle timeout or SIGINT is  
analagous, I think.


...Robert

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

[HACKERS] Win64 warnings about size_t

I have adapted the win64 patches a bit, and now have a working build.
As in it runs the regression tests fine. However, I have well over a
thousand warnings of the type:
conversion from 'size_t' to 'int', possible loss of data

My first 5-6 checks of where these happen are all cases where we
assign the result of strlen() something to an int, or call a function
taking an int as parameter with the result of strlen() in there.

strlen() returns size_t, which AFAICS is per the standard and not even
a Microsoft-specific idea. size_t is 8-bit - but it appears to be
8-bit on Linux as well, when in 64-bit mode.

So I don't really see what win64 does differently in this case, but
perhaps I've been looking at this code too long? Or is it simply that
MSVC warns about this and GCC doesn't, and I shuld disbale the
warning?

A couple of points for reference:

1.\src\backend\utils\adt\varlena.c(84) : warning C4267: 'function' :
conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(158) : warning C4267: 'function' :
conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(158) : warning C4267: 'function' :
conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(201) : warning C4267: '=' :
conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(203) : warning C4267: 'function' :
conversion from 'size_t' to 'unsigned int', possible loss of data
1.\src\backend\utils\adt\varlena.c(488) : warning C4267: 'function' :
conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(2156) : warning C4267: 'function'
: conversion from 'size_t' to 'int', possible loss of data
1.\src\backend\utils\adt\varlena.c(3131) : warning C4267: '=' :
conversion from 'size_t' to 'int32', possible loss of data
1.\src\backend\utils\adt\varlena.c(3136) : warning C4267: '=' :
conversion from 'size_t' to 'int32', possible loss of data


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

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

Re: [HACKERS] Cancelling idle in transaction state

On Fri, 2010-01-01 at 17:14 +0200, Heikki Linnakangas wrote:

  Which amounts to rejecting this patch, since *this* patch changes the
  behaviour of SIGINT for all senders, which is what I understood people
  desired, i.e. not just a change for Hot Standby. I assumed Joachim did
  not mean to veto his own patch, but I'm not sure what you think here?
  (I don't mind either way).
 
 I don't know, I don't feel strongly about this. Is there really no other
 way?

Multiple behaviours on signal implies multiplexing, AFAICS.

Question on the table is: Should SIGINT be extended to cancel an
idle-in-transaction session, or not? I'll wait a little while longer
before committing this to make sure I have the full spread of opinion.

Tom's opinion was...

On Tue, 2009-12-29 at 10:22 -0500, Tom Lane wrote: 
 Joachim Wieland j...@mcknight.de writes:
  If we use the same signal for both cases, the receiving backend cannot
  tell what the intention of the sending backend was. That's why I
  proposed to make SIGINT similar to SIGUSR1 where we write a reason to
  a shared memory structure first and then send the signal (see
  http://archives.postgresql.org/pgsql-hackers/2009-12/msg02067.php from
  a few days ago).
 
 This seems like a fairly bad idea.  One of the intended use-cases is to
 be able to manually kill -INT a misbehaving backend.  Assuming that
 there will be valid info about the signal in shared memory will break
 that.

So it seems that we have at least one vote in favour of making SIGINT
blow anything away, no matter what its state. 

I support that also, but I don't need it for HS, its just an objective
opinion. So that's plus 2, unsure about Joachim. Any others?

 Seems useful to me, so that you know why your transaction was cancelled.
 It's rather weird to see no ERRORs in the previous steps, and suddenly
 you see that the transaction is aborted. And none your savepoints exist
 anymore either.

I agree we need a message to explain, it just seems wrong to me to do
this in a way that appears to accentuate this particular source of error
over similar sources.

However, I will do as requested, though will leave existing error
sources alone.

-- 
 Simon Riggs   www.2ndQuadrant.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] Cancelling idle in transaction state

On Fri, 2010-01-01 at 09:24 -0800, Robert Haas wrote:

  If we have other events that can asynchronously roll back a
  transaction, I would think they would deserve similar handling.  Off
  the top of my head, I'm not sure if there are any such cases.
 
  Serialization failures, deadlocks, timeouts, SIGINT, out of memory
  errors etc..
 
 Hmm. I don't think I can get a serialization failure, deadlock, or out  
 of memory error while my session is idle. 

Agreed. As a point of note, now that we can cancel idle transactions
there isn't any future blocker from making serialization failures or
deadlocks cancel such transactions... Other RDBMS have deadlock
detectors that can pick any session to resolve, not just the one doing
the deadlock checking.

 An idle timeout or SIGINT is analagous, I think.

-- 
 Simon Riggs   www.2ndQuadrant.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] Win64 warnings about size_t

Magnus Hagander mag...@hagander.net writes:
 My first 5-6 checks of where these happen are all cases where we
 assign the result of strlen() something to an int, or call a function
 taking an int as parameter with the result of strlen() in there.

Yeah.  Getting rid of all those cases is impractical I think, and
pointless anyway --- we have limitations in palloc and Datum
representation that ensure we'll never be dealing with strings (or other
values) larger than 1GB.

 strlen() returns size_t, which AFAICS is per the standard and not even
 a Microsoft-specific idea.

Correct.

 So I don't really see what win64 does differently in this case, but
 perhaps I've been looking at this code too long? Or is it simply that
 MSVC warns about this and GCC doesn't, and I shuld disbale the
 warning?

I think MSVC is just complaining about something gcc doesn't.  If you
can disable this specific warning it'd be a good plan.

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] Win64 warnings about size_t

On Fri, Jan 1, 2010 at 18:47, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 My first 5-6 checks of where these happen are all cases where we
 assign the result of strlen() something to an int, or call a function
 taking an int as parameter with the result of strlen() in there.

 Yeah.  Getting rid of all those cases is impractical I think, and
 pointless anyway --- we have limitations in palloc and Datum
 representation that ensure we'll never be dealing with strings (or other
 values) larger than 1GB.

Ok.


 strlen() returns size_t, which AFAICS is per the standard and not even
 a Microsoft-specific idea.

 Correct.

 So I don't really see what win64 does differently in this case, but
 perhaps I've been looking at this code too long? Or is it simply that
 MSVC warns about this and GCC doesn't, and I shuld disbale the
 warning?

 I think MSVC is just complaining about something gcc doesn't.  If you
 can disable this specific warning it'd be a good plan.

Yeah, that should be doable. According to
http://msdn.microsoft.com/en-us/library/6kck0s93(VS.80).aspx this
warning is only about size_t, and not about the issue in general, so I
can just disable C4267 and they should go away.

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

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

Re: [HACKERS] Win64 warnings about size_t

Magnus Hagander mag...@hagander.net writes:
 On Fri, Jan 1, 2010 at 18:47, Tom Lane t...@sss.pgh.pa.us wrote:
 I think MSVC is just complaining about something gcc doesn't.  If you
 can disable this specific warning it'd be a good plan.

 Yeah, that should be doable. According to
 http://msdn.microsoft.com/en-us/library/6kck0s93(VS.80).aspx this
 warning is only about size_t, and not about the issue in general, so I
 can just disable C4267 and they should go away.

Sounds good.  The case we do need to look for is any remaining casts
between pointer and long.  If that's a different warning number then
we'll be in good shape.

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] Cancelling idle in transaction state

2010-01-01 Thread Robert Haas


On Jan 1, 2010, at 9:42 AM, Simon Riggs si...@2ndquadrant.com wrote:

On Fri, 2010-01-01 at 09:24 -0800, Robert Haas wrote:


If we have other events that can asynchronously roll back a
transaction, I would think they would deserve similar handling.   
Off

the top of my head, I'm not sure if there are any such cases.


Serialization failures, deadlocks, timeouts, SIGINT, out of memory
errors etc..


Hmm. I don't think I can get a serialization failure, deadlock, or  
out

of memory error while my session is idle.


Agreed. As a point of note, now that we can cancel idle transactions
there isn't any future blocker from making serialization failures or
deadlocks cancel such transactions... Other RDBMS have deadlock
detectors that can pick any session to resolve, not just the one doing
the deadlock checking.


Interesting. It's not obvious to me how killing an *idle* session can  
resolve a deadlock. AIUI a deadlock requires a cycle in the waits-for  
graph, and an idle transaction is not waiting for a lock acquisition.   
I can see how it could be useful in handling serialization failures,  
though, and there may be other applications as well.  This is a nice  
improvement; I'm pleased to see it going in.


...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] Serializable Isolation without blocking

2010-01-01 Thread Albe Laurenz

Kevin Grittner wrote:
 It seems to me that the hard part of this problem is to describe
 the general mechanism by which conflicts will be detected, with
 specific references to the types of data structures that will be
 used to hold that information.

 Well, the general approach to tracking SIREAD locks I had in mind is
 to keep them in the existing lock data structures.
 [...]
 
If I remember right, one necessity for the SIREAD lock technique was
that SIREAD locks taken by a transaction have to be kept after the
transaction has ended.
Won't that require fundamental changes?
 
Yours,
Laurenz Albe

-- 
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] Cancelling idle in transaction state

On Fri, 2010-01-01 at 10:15 -0800, Robert Haas wrote:
  Hmm. I don't think I can get a serialization failure, deadlock, or  
  out
  of memory error while my session is idle.
 
  Agreed. As a point of note, now that we can cancel idle transactions
  there isn't any future blocker from making serialization failures or
  deadlocks cancel such transactions... Other RDBMS have deadlock
  detectors that can pick any session to resolve, not just the one doing
  the deadlock checking.
 
 Interesting. It's not obvious to me how killing an *idle* session can  
 resolve a deadlock. AIUI a deadlock requires a cycle in the waits-for  
 graph, and an idle transaction is not waiting for a lock acquisition.   

In strict theory, yes.

In practice, many lock contention situations are caused by long running
idle transactions, so having a deadlock detector be able to resolve a
situation by deciding that an idle xact is actually in some kind of wait
state would be very useful.

Some people have asked for a idle-in-transaction-timeout. I would be
more inclined to have a settable time after which an idle-in-transaction
session that blocks an active lock requestor can be aborted by the
deadlock detector as a way of resolving a lock wait. Idle-in-transaction
sessions that don't hold any locks aren't the same kind of annoyance,
though there may be other reasons to remove them.

-- 
 Simon Riggs   www.2ndQuadrant.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] Thoughts on statistics for continuously advancing columns

On Thu, 2009-12-31 at 21:29 +, Simon Riggs wrote:
 On Thu, 2009-12-31 at 15:18 -0500, Tom Lane wrote:
  Simon Riggs si...@2ndquadrant.com writes:
   Why not get both max() and min(), then rebase the histogram according to
   those values. That way the histogram can still move significantly and
   the technique will still work.
  
  Define rebase, keeping in mind that this has to work on datatypes that
  we don't have a distance metric for.
 
 Make it work differently according to whether we have, or not, just as
 we do elsewhere with stats. No point in limiting ourselves to the lowest
 common denominator, especially when the common case is integer keys and
 time datatypes.

This seemed obvious but I understand now that you meant we don't know
that from the datatype definition, so we can't do as I suggested, yet.

-- 
 Simon Riggs   www.2ndQuadrant.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] uintptr_t for Datum

On tor, 2009-12-31 at 13:44 -0500, Bruce Momjian wrote:
 Tom Lane wrote:
  Bruce Momjian br...@momjian.us writes:
   Tom Lane wrote:
   BTW, it looks like the patch is showing a manual change to
   pg_config.h.in.  Don't do that.  Run autoheader.
  
   I wasn't aware autoheader existed.  Is that new or has it alwasy been
   part of autoconf?
  
  It's always been there, or at least for many years.  pg_config.h.in
  really ought to be thought of the same as configure: you don't edit
  it, you just generate it.
 
 Well, that's pretty confusing considering it has a .in suffix, just like
 configure.in, which we do edit, but I get your point.

I realize it's easy to miss, but the first line of pg_config.h.in tells
that it is generated.


-- 
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] Win64 warnings about size_t

On Fri, Jan 1, 2010 at 18:59, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Fri, Jan 1, 2010 at 18:47, Tom Lane t...@sss.pgh.pa.us wrote:
 I think MSVC is just complaining about something gcc doesn't.  If you
 can disable this specific warning it'd be a good plan.

 Yeah, that should be doable. According to
 http://msdn.microsoft.com/en-us/library/6kck0s93(VS.80).aspx this
 warning is only about size_t, and not about the issue in general, so I
 can just disable C4267 and they should go away.

 Sounds good.  The case we do need to look for is any remaining casts
 between pointer and long.  If that's a different warning number then
 we'll be in good shape.

I don't see any such warnings at all. I do see this however:

  .\src\backend\utils\mmgr\aset.c(701): warning C4334: '' : result
of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
intended?)
  .\src\backend\utils\mmgr\aset.c(705): warning C4334: '' : result
of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
intended?)

Perhaps we need some casting there?

Other than that, I see a few more API related warnings that are not
related directly to 32/64-bit. I'l be working on those.


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

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

Re: [HACKERS] PATCH: Add hstore_to_json()

On tor, 2009-12-31 at 11:12 -0500, Andrew Dunstan wrote:
 
 David E. Wheeler wrote:
  On Dec 31, 2009, at 1:04 AM, Peter Eisentraut wrote:
 

  I think the primary use will be to load a JSON value into Perl or Python
  and process it there.  So a json type that doesn't have any interesting
  operators doesn't sound useless to me.  The features I would like to get
  out of it are input validation and encoding handling and smooth
  integration with said languages.
  
 
  What about access to various parts of a JSON data structure? Or is that 
  just asking for too much trouble up-front?

 IMNSHO it's essential. I think Peter's approach of ignoring this 
 requirement is extremely shortsighted.

Whose requirement is it?  I'm not ignoring it, but so far no one has
actually said that it is a requirement and why.


-- 
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] Win64 warnings about size_t

On fre, 2010-01-01 at 20:01 +0100, Magnus Hagander wrote:
   .\src\backend\utils\mmgr\aset.c(701): warning C4334: '' : result
 of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
 intended?)
   .\src\backend\utils\mmgr\aset.c(705): warning C4334: '' : result
 of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
 intended?)
 
 Perhaps we need some casting there?

This shouldn't be a problem for the same reason that casting size_t to
int is not a problem in the PostgreSQL backend code, but perhaps writing
1L  ... would fix it.


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

[HACKERS] win32 socket definition

The win64 port has showed that we have two sockets declared
incorrectly. They are supposed to be declared as SOCKET on win32, but
they are declared as int. See attached patch.

Given that SOCKET is actually defined as int on win32 (no warnings or
anything there, just on win64), I'm inclined to apply this patch just
to HEAD and not bother with backpatching.

Comments?

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


win32_socket.patch
Description: Binary data

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

Re: [HACKERS] Win64 warnings about size_t

On Fri, Jan 1, 2010 at 20:12, Peter Eisentraut pete...@gmx.net wrote:
 On fre, 2010-01-01 at 20:01 +0100, Magnus Hagander wrote:
   .\src\backend\utils\mmgr\aset.c(701): warning C4334: '' : result
 of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
 intended?)
   .\src\backend\utils\mmgr\aset.c(705): warning C4334: '' : result
 of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
 intended?)

 Perhaps we need some casting there?

 This shouldn't be a problem for the same reason that casting size_t to
 int is not a problem in the PostgreSQL backend code, but perhaps writing
 1L  ... would fix it.

1L didn't fix it. 1LL did, however.

ISTM that this is a warning we don't want to disable, so assuming that
should be safe on other platforms (it passes on my 32-bit linux
without warnings or anything), I'd vote for putting it in there.

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

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

Re: [HACKERS] Win64 warnings about size_t

Magnus Hagander mag...@hagander.net writes:
 On Fri, Jan 1, 2010 at 20:12, Peter Eisentraut pete...@gmx.net wrote:
 This shouldn't be a problem for the same reason that casting size_t to
 int is not a problem in the PostgreSQL backend code, but perhaps writing
 1L  ... would fix it.

 1L didn't fix it. 1LL did, however.

... and would break things on many many other platforms.

Use (Size) 1 instead of 1 if you really want to suppress this.

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] win32 socket definition

Magnus Hagander mag...@hagander.net writes:
 The win64 port has showed that we have two sockets declared
 incorrectly. They are supposed to be declared as SOCKET on win32, but
 they are declared as int. See attached patch.

 Given that SOCKET is actually defined as int on win32 (no warnings or
 anything there, just on win64), I'm inclined to apply this patch just
 to HEAD and not bother with backpatching.

This looks pretty bletcherous --- plastering #ifdef WIN32 all over the
code is exactly not the way to be fixing this sort of thing.  Maybe we
should go the other direction of typedef int SOCKET on Unix then use
SOCKET everywhere.

BTW, isn't this porting project showing the shortsightedness of using
WIN32 as the its-Windows platform symbol?  The case that you're
worried about here is certainly not WIN32.

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] Windows x64 [repost]

On Fri, Dec 4, 2009 at 11:42, Tsutomu Yamada tsut...@sraoss.co.jp wrote:

 2) use appropriate macro and datatypes for Windows API.
   enables more than 32bits shared memory.

Are you sure this one should use __noop, and not __nop?

__noop: http://msdn.microsoft.com/en-us/library/s6btaxcs.aspx
__nop: http://msdn.microsoft.com/en-us/library/aa983381.aspx

I think __nop is what we want?

Also, that turns it into nop and not rep nop, no?

Should we perhaps instead use __yield, per:
http://msdn.microsoft.com/en-us/library/2b2h26kx.aspx


Hopefully someone who knows this better than me can comment :-)

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

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

Re: [HACKERS] Win64 warnings about size_t

On Fri, Jan 1, 2010 at 20:33, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Fri, Jan 1, 2010 at 20:12, Peter Eisentraut pete...@gmx.net wrote:
 This shouldn't be a problem for the same reason that casting size_t to
 int is not a problem in the PostgreSQL backend code, but perhaps writing
 1L  ... would fix it.

 1L didn't fix it. 1LL did, however.

 ... and would break things on many many other platforms.

That's what I was afraid of.


 Use (Size) 1 instead of 1 if you really want to suppress this.

That fixes it as well. Will apply a patch to that effect.


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

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

Re: [HACKERS] IntArray in c.h

On tor, 2009-12-31 at 11:28 -0500, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  The definition of c.h is bogus anyway.  You might think it contains
  includes and defines to set up a portable C environment, which is what
  the first half indeed does.
 
  But then things like regproc, transaction ID types, IntArray, varlena,
  bytea, oidvector, NameData, etc. do not belong there and should be moved
  to postgres.h.
 
 Actually, what c.h does is to provide definitions that are needed in
 both frontend and backend code.  And we do NOT want to start including
 postgres.h in frontend code.  It might be that some of the declarations
 there are useless to frontend code and could be moved, but trying to be
 as strict as you suggest is only going to create problems.

I think the list above is a pretty good list of things that client code
doesn't need, plus or minus a few things maybe.



-- 
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] win32 socket definition

On Fri, Jan 1, 2010 at 20:41, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 The win64 port has showed that we have two sockets declared
 incorrectly. They are supposed to be declared as SOCKET on win32, but
 they are declared as int. See attached patch.

 Given that SOCKET is actually defined as int on win32 (no warnings or
 anything there, just on win64), I'm inclined to apply this patch just
 to HEAD and not bother with backpatching.

 This looks pretty bletcherous --- plastering #ifdef WIN32 all over the
 code is exactly not the way to be fixing this sort of thing.  Maybe we
 should go the other direction of typedef int SOCKET on Unix then use
 SOCKET everywhere.

Yeah, we can do that - I figured since it was only two places, this
was easier...

In keeping with how we usually name things though, shouldn't it be
pg_socket, and then have it typdef'ed to two different things
depending on which platform you're on?


 BTW, isn't this porting project showing the shortsightedness of using
 WIN32 as the its-Windows platform symbol?  The case that you're
 worried about here is certainly not WIN32.

Well, the API is actually called Win32 for 64-bit Windows these
days. And the vast majority of the APIs are exactly the same. With the
patch that I'm working off now, there are exactly 4 #ifdef _WIN64
sections, two of which are in pg_config.h.win32 (which is intended to
deal with such things). The other one is spinlock and shared memory.
Everything else is exactly the same code.

The socket thing specifically, is already wrong on 32-bit win32. It's
just that it didn't throw a warning then.

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

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

Re: [HACKERS] Serializable Isolation without blocking

2010-01-01 Thread Kevin Grittner

Albe Laurenz  wrote:
 
 If I remember right, one necessity for the SIREAD lock technique
 was that SIREAD locks taken by a transaction have to be kept after
 the transaction has ended.
 
Correct.  An SIREAD lock must be held until completion of the last
serializable transaction holding a snapshot earlier than the lock
transaction's commit.

Exceptions to that are:
 
- If a serializable transaction is rolled back, all SIREAD locks can
  be immediately released.
 
- If an an ACCESS EXCLUSIVE lock is acquired on the exact same object
  already locked by an SIREAD lock, the SIREAD lock can be released.
  (This is an optimization, not a requirement for correctness.)
 
 Won't that require fundamental changes?
 
We already have two different lock methods, one of which already
keeps locks past the end of a database transaction.  USER locks
persist until the session terminates or the lock is explicitly
released.  Given that, I don't think it would be fair to characterize
a third lock method with a third lifespan as a fundamental change --
just another implementation detail.
 
-Kevin

-- 
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] Cancelling idle in transaction state

2010-01-01 Thread Kris Jurka




On Thu, 31 Dec 2009, Simon Riggs wrote:


On Thu, 2009-12-31 at 15:41 +0100, Joachim Wieland wrote:


I still think that we should have three transaction cancel modes, one
to cancel an idle transaction, another one to cancel a running query
and a third one that just cancels the transaction regardless of it
being idle or not. This last one is what you are implementing now, and
it is what HS wants to do.


pg_cancel_backend() is currently conditional on whether a statement is
active or not, so should really be called pg_cancel_if_active(). What
people want is an unconditional way to stop a transaction. I don't see
the need for 3 modes (and that has nothing to do with HS).



The JDBC driver does want cancel if active behavior.  The JDBC API 
specifies Statement.cancel() where Statement is running one particular 
backend query.  So it really does want to cancel just that one query. 
Already this is tough because of the asynchronous nature of the cancel 
protocol and the inability to say exactly what should be cancelled.


Kris Jurka

--
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] Cancelling idle in transaction state

On Fri, 2010-01-01 at 15:31 -0500, Kris Jurka wrote:
 
 On Thu, 31 Dec 2009, Simon Riggs wrote:
 
  On Thu, 2009-12-31 at 15:41 +0100, Joachim Wieland wrote:
 
  I still think that we should have three transaction cancel modes, one
  to cancel an idle transaction, another one to cancel a running query
  and a third one that just cancels the transaction regardless of it
  being idle or not. This last one is what you are implementing now, and
  it is what HS wants to do.
 
  pg_cancel_backend() is currently conditional on whether a statement is
  active or not, so should really be called pg_cancel_if_active(). What
  people want is an unconditional way to stop a transaction. I don't see
  the need for 3 modes (and that has nothing to do with HS).
 
 
 The JDBC driver does want cancel if active behavior.  The JDBC API 
 specifies Statement.cancel() where Statement is running one particular 
 backend query.  So it really does want to cancel just that one query. 
 Already this is tough because of the asynchronous nature of the cancel 
 protocol and the inability to say exactly what should be cancelled.

OK, I think that is conclusive.

CancelRequest's behaviour currently equates to SIGINT, so
processCancelRequest() can only use SIGINT if SIGINT's behaviour remains
same.

I would recommend we make SIGINT do cancel-anything, and handle
everything else via SIGUSR1, including CancelRequest. I'm not going to
do that; I'm going to make HS conflict resolution work, which means
putting in enough infrastructure to allow someone else to make SIGINT
changes work at a later time, if appropriate.

-- 
 Simon Riggs   www.2ndQuadrant.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: Add hstore_to_json()

2010-01-01 Thread Andrew Dunstan




Peter Eisentraut wrote:
IMNSHO it's essential. I think Peter's approach of ignoring this 
requirement is extremely shortsighted.



Whose requirement is it?  I'm not ignoring it, but so far no one has
actually said that it is a requirement and why.

  


Mine for one :-). Quite apart from any other reason I would expect it to 
make indexing parts of the JSON more tractable. Say we use it to store a 
web session object, which is a natural enough use. I might well want to 
find or modify sessions with certain characteristics. I'm sure I 
wouldn't be the only possible usewr who would want something 
substantially more of such a type than just being able to validate it. 
We have XPath for XML. and a substantial accessor API for hstore, so why 
would we want anything less for JSON?


In general we have adopted an approach that allows for a very rich type 
system, with a substantial set of manipulator functions for almost all 
types. That's one of the things I find attractive about Postgres, so I 
think we should stick to it in this instance.


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] PATCH: Add hstore_to_json()