Fix issues with checks for unsupported transaction states in Hot Standby.

The GUC check hooks for transaction_read_only and transaction_isolation
tried to check RecoveryInProgress(), so as to disallow setting read/write
mode or serializable isolation level (respectively) in hot standby
sessions.  However, GUC check hooks can be called in many situations where
we're not connected to shared memory at all, resulting in a crash in
RecoveryInProgress().  Among other cases, this results in EXEC_BACKEND
builds crashing during child process start if default_transaction_isolation
is serializable, as reported by Heikki Linnakangas.  Protect those calls
by silently allowing any setting when not inside a transaction; which is
okay anyway since these GUCs are always reset at start of transaction.

Also, add a check to GetSerializableTransactionSnapshot() to complain
if we are in hot standby.  We need that check despite the one in
check_XactIsoLevel() because default_transaction_isolation could be
serializable.  We don't want to complain any sooner than this in such
cases, since that would prevent running transactions at all in such a
state; but a transaction can be run, if SET TRANSACTION ISOLATION is done
before setting a snapshot.  Per report some months ago from Robert Haas.

Back-patch to 9.1, since these problems were introduced by the SSI patch.

Kevin Grittner and Tom Lane, with ideas from Heikki Linnakangas

diff --git a/src/backend/commands/variable.c b/src/backend/commands/variable.c
index 8550869db3d959f0d403e3c165740bc15b02ddd3..3eb911c5cc57abdaeff838df4a1bc8305b87dde4 100644 (file)
--- a/src/backend/commands/variable.c
+++ b/src/backend/commands/variable.c
@@ -569,11 +569,16 @@ show_log_timezone(void)
  * read-only may be changed to read-write only when in a top-level transaction
  * that has not yet taken an initial snapshot. Can't do it in a hot standby
  * slave, either.
+ *
+ * If we are not in a transaction at all, just allow the change; it means
+ * nothing since XactReadOnly will be reset by the next StartTransaction().
+ * The IsTransactionState() test protects us against trying to check
+ * RecoveryInProgress() in contexts where shared memory is not accessible.
  */
 bool
 check_transaction_read_only(bool *newval, void **extra, GucSource source)
 {
-       if (*newval == false && XactReadOnly)
+       if (*newval == false && XactReadOnly && IsTransactionState())
        {
                /* Can't go to r/w mode inside a r/o transaction */
                if (IsSubTransaction())
@@ -592,6 +597,7 @@ check_transaction_read_only(bool *newval, void **extra, GucSource source)
                /* Can't go to r/w mode while recovery is still active */
                if (RecoveryInProgress())
                {
+                       GUC_check_errcode(ERRCODE_FEATURE_NOT_SUPPORTED);
                        GUC_check_errmsg("cannot set transaction read-write mode during recovery");
                        return false;
                }
@@ -605,6 +611,8 @@ check_transaction_read_only(bool *newval, void **extra, GucSource source)
  *
  * We allow idempotent changes at any time, but otherwise this can only be
  * changed in a toplevel transaction that has not yet taken a snapshot.
+ *
+ * As in check_transaction_read_only, allow it if not inside a transaction.
  */
 bool
 check_XactIsoLevel(char **newval, void **extra, GucSource source)
@@ -634,7 +642,7 @@ check_XactIsoLevel(char **newval, void **extra, GucSource source)
        else
                return false;
 
-       if (newXactIsoLevel != XactIsoLevel)
+       if (newXactIsoLevel != XactIsoLevel && IsTransactionState())
        {
                if (FirstSnapshotSet)
                {
@@ -652,6 +660,7 @@ check_XactIsoLevel(char **newval, void **extra, GucSource source)
                /* Can't go to serializable mode while recovery is still active */
                if (newXactIsoLevel == XACT_SERIALIZABLE && RecoveryInProgress())
                {
+                       GUC_check_errcode(ERRCODE_FEATURE_NOT_SUPPORTED);
                        GUC_check_errmsg("cannot use serializable mode in a hot standby");
                        GUC_check_errhint("You can use REPEATABLE READ instead.");
                        return false;

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 532dedf959bd2d262a38fb7cbcad8738c9fe0ecb..56f5fb31feeebf978fd73d697cfbe34d83eca46f 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1558,6 +1558,19 @@ RegisterSerializableTransaction(Snapshot snapshot)
 {
        Assert(IsolationIsSerializable());
 
+       /*
+        * Can't use serializable mode while recovery is still active, as it is,
+        * for example, on a hot standby.  We could get here despite the check
+        * in check_XactIsoLevel() if default_transaction_isolation is set to
+        * serializable, so phrase the hint accordingly.
+        */
+       if (RecoveryInProgress())
+               ereport(ERROR,
+                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot use serializable mode in a hot standby"),
+                                errdetail("\"default_transaction_isolation\" is set to \"serializable\"."),
+                                errhint("You can use \"SET default_transaction_isolation = 'repeatable read'\" to change the default.")));
+
        /*
         * A special optimization is available for SERIALIZABLE READ ONLY
         * DEFERRABLE transactions -- we can wait for a suitable snapshot and

diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 94f92dd7343af74c37b7efe9f5f57a44f42c3d9e..dfe338c2bfefe9d7a8d3a918dbd760e877b5da9c 100644 (file)
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -574,6 +574,15 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
                /* statement_timestamp must be set for timeouts to work correctly */
                SetCurrentStatementStartTimestamp();
                StartTransactionCommand();
+
+               /*
+                * transaction_isolation will have been set to the default by the
+                * above.  If the default is "serializable", and we are in hot
+                * standby, we will fail if we don't change it to something lower.
+                * Fortunately, "read committed" is plenty good enough.
+                */
+               XactIsoLevel = XACT_READ_COMMITTED;
+
                (void) GetTransactionSnapshot();
        }