Re: [HACKERS] SQLSTATE for Hot Standby cancellation

2010-05-07 Thread Simon Riggs
On Thu, 2010-05-06 at 15:00 +0100, Simon Riggs wrote:
 On Thu, 2010-05-06 at 12:08 +0200, Yeb Havinga wrote:
 
  That's funny because when I was reading this thread, I was thinking the 
  exact opposite: having max_standby_delay always set to 0 so I know the 
  standby server is as up-to-date as possible. The application that 
  accesses the hot standby has to be 'special' anyway because it might 
  deliver not-up-to-date data. If that information about specialties 
  regarding querying the standby server includes the warning that queries 
  might get cancelled, they can opt for a retry themselves (is there a 
  special return code to catch that case? like PGRES_RETRY_LATER) or a 
  message to the user that their report is currently unavailable and they 
  should retry in a few minutes.
 
 Very sensible. Kevin Grittner already asked for that and I alread
 agreed, yet it has not been implemented that way
 http://archives.postgresql.org/pgsql-hackers/2008-12/msg01691.php
 
 Can anyone remember a specific objection to that 'cos it still sounds
 very sensible to me and is a quick, low impact change.
 
 Currently the SQLSTATE is ERRCODE_ADMIN_SHUTDOWN or
 ERRCODE_QUERY_CANCELED if not idle. That makes it even harder to
 determine the retryability, since it can come back in one of two states.
 
 Proposed SQLSTATE for HS cancellation is 
  case PROCSIG_RECOVERY_CONFLICT_BUFFERPIN:
  case PROCSIG_RECOVERY_CONFLICT_LOCK:
  case PROCSIG_RECOVERY_CONFLICT_SNAPSHOT:
  case PROCSIG_RECOVERY_CONFLICT_STARTUP_DEADLOCK:
   recovery_conflict_errcode = ERRCODE_T_R_SERIALIZATION_FAILURE;
   break;
  case PROCSIG_RECOVERY_CONFLICT_DATABASE:
  case PROCSIG_RECOVERY_CONFLICT_TABLESPACE:
   recovery_conflict_errcode = ERRCODE_ADMIN_SHUTDOWN;
   break;
 
 We don't have a retryable SQLSTATE, so perhaps we should document that
 serialization_failure and deadlock_detected are both retryable error
 conditions. As the above implies HS can throw some errors that are
 non-retyable, so we use ERRCODE_ADMIN_SHUTDOWN.

Implemented as ERRCODE_ADMIN_SHUTDOWN only in the case of a dropped database.
ERRCODE_T_R_SERIALIZATION_FAILURE in other cases.

-- 
 Simon Riggs   www.2ndQuadrant.com
*** a/src/backend/tcop/postgres.c
--- b/src/backend/tcop/postgres.c
***
*** 2936,2949  ProcessInterrupts(void)
  			DisableCatchupInterrupt();
  			if (DoingCommandRead)
  ereport(FATAL,
! 		(errcode(ERRCODE_ADMIN_SHUTDOWN),
  		 errmsg(terminating connection due to conflict with recovery),
  		 errdetail_recovery_conflict(),
   errhint(In a moment you should be able to reconnect to the
  		  database and repeat your command.)));
  			else
  ereport(ERROR,
! 		(errcode(ERRCODE_QUERY_CANCELED),
   errmsg(canceling statement due to conflict with recovery),
  		 errdetail_recovery_conflict()));
  		}
--- 2936,2949 
  			DisableCatchupInterrupt();
  			if (DoingCommandRead)
  ereport(FATAL,
! 		(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  		 errmsg(terminating connection due to conflict with recovery),
  		 errdetail_recovery_conflict(),
   errhint(In a moment you should be able to reconnect to the
  		  database and repeat your command.)));
  			else
  ereport(ERROR,
! 		(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
   errmsg(canceling statement due to conflict with recovery),
  		 errdetail_recovery_conflict()));
  		}

-- 
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] SQLSTATE for Hot Standby cancellation

2010-05-06 Thread Kevin Grittner
Simon Riggs si...@2ndquadrant.com wrote:
 
 We don't have a retryable SQLSTATE, so perhaps we should document
 that serialization_failure and deadlock_detected are both
 retryable error conditions. As the above implies HS can throw some
 errors that are non-retyable, so we use ERRCODE_ADMIN_SHUTDOWN.
 
There is one SQLSTATE (40001 - serialization_failure) to indicate
that the transaction failed due to conflicts with concurrent
transactions.  This is supposed to be used when there was no problem
with the transaction itself, were it to be run when nothing else is
running and the environment isn't failing.  (For example, it isn't
the right code to use if you fail because your network connection is
broken or you're running out of disk space.)  In my view it is a
mistake *not* to use it whenever concurrent transactions cause
failure; we can always use different error message text, details,
and hints to differentiate between the particular ways in which
concurrent transactions caused the failure.
 
The advantage of consistently using the standard value for this is
that there is software which recognizes this and automatically
retries the transaction from the beginning in a manner which is
transparent to the users and the application code.  Our users never
see anything but a delay when hitting this; it's effectively the
same as blocking, with no application programmer effort needed.
 
Having a separate deadlock_detected SQLSTATE is arguably a bug.  (In
fact, I have argued that; but haven't persuaded the community of
it.)  Coming up with yet another SQLSTATE to indicate that there's
nothing wrong with the transaction except its timing in relation to
other transactions would, in my view, compound the error.
 
That said, our shop is resourceful enough (in more than one sense)
to overcome this if need be.  I just don't see why the community
would choose to create barriers which cause failures for software
which would otherwise Just Work, and the corresponding programming
burden on shops using PostgreSQL to keep a list of nothing really
wrong with the transaction, you can probably just reschedule it
SQLSTATE values for the product.
 
-Kevin

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