ERRCODE_READ_ONLY_SQL_TRANSACTION
Hot Standby returns ERRCODE_READ_ONLY_SQL_TRANSACTION in most cases
for illegal actions on a standby.
There are two possible but not normally seen cases that give errors,
but don't set the correct sqlstate, which makes it difficult to
diagnose misdirected SQL from more normal SQL problems.
*Patch corrects this. Thanks to Dimitri for the report.
Backpatching to 9.0
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
ERRCODE_READ_ONLY_SQL_TRANSACTION.v1.patchtext/x-patch; charset=US-ASCII; name=ERRCODE_READ_ONLY_SQL_TRANSACTION.v1.patchDownload
diff --git a/src/backend/access/transam/varsup.c b/src/backend/access/transam/varsup.c
index 8c045fb..62031eb 100644
--- a/src/backend/access/transam/varsup.c
+++ b/src/backend/access/transam/varsup.c
@@ -60,7 +60,9 @@ GetNewTransactionId(bool isSubXact)
/* safety check, we should never get this far in a HS slave */
if (RecoveryInProgress())
- elog(ERROR, "cannot assign TransactionIds during recovery");
+ ereport(ERROR,
+ (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION),
+ errmsg("cannot assign TransactionIds during recovery")));
LWLockAcquire(XidGenLock, LW_EXCLUSIVE);
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 19ef66b..8ac194a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -713,7 +713,9 @@ XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata)
/* cross-check on whether we should be here or not */
if (!XLogInsertAllowed())
- elog(ERROR, "cannot make new WAL entries during recovery");
+ ereport(ERROR,
+ (errcode(ERRCODE_READ_ONLY_SQL_TRANSACTION),
+ errmsg("cannot make new WAL entries during recovery")));
/* info's high bits are reserved for use by me */
if (info & XLR_INFO_MASK)
Simon Riggs <simon@2ndQuadrant.com> writes:
Hot Standby returns ERRCODE_READ_ONLY_SQL_TRANSACTION in most cases
for illegal actions on a standby.
There are two possible but not normally seen cases that give errors,
but don't set the correct sqlstate, which makes it difficult to
diagnose misdirected SQL from more normal SQL problems.
*Patch corrects this. Thanks to Dimitri for the report.
I don't think I like this patch: you are promoting what are and ought to
be very low-level internal sanity checks into user-facing errors (which
among other things will require translation effort for the messages).
I note that you didn't bother even to adjust the adjacent comments
saying this should never happen.
What I want to know is what code path led to these and why the read-only
ereport did not occur at a far higher level. If we have gaps in the RO
checking we should be fixing them somewhere else, not band-aiding here.
regards, tom lane
Hi,
Tom Lane <tgl@sss.pgh.pa.us> writes:
Simon Riggs <simon@2ndQuadrant.com> writes:
Hot Standby returns ERRCODE_READ_ONLY_SQL_TRANSACTION in most cases
for illegal actions on a standby.I don't think I like this patch: you are promoting what are and ought to
be very low-level internal sanity checks into user-facing errors (which
among other things will require translation effort for the messages).
So it seems the last-9-2-CF deadline is making us a little too hasty.
Apparently as you're saying there's no way to exercise that code paths
from an SQL connection on a Hot Standby short of deploying a C coded
extension calling either GetNewTransactionId() or XLogInsert(), which
means it's out of scope.
My quest was figuring out if ERRCODE_READ_ONLY_SQL_TRANSACTION really is
trustworthy as a signal that you could transparently now redirect the
transaction to the master when seeing that in a “proxy” of some sort.
I felt that we were missing something simple here, but after review I
think we finally have all the pieces to achieve that with current 9.2
code base in fact.
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Fri, Jan 13, 2012 at 8:55 AM, Dimitri Fontaine
<dimitri@2ndquadrant.fr> wrote:
I felt that we were missing something simple here, but after review I
think we finally have all the pieces to achieve that with current 9.2
code base in fact.
Good, patch revoked. No time wasted, it was worth checking.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services