On 11 June 2016 at 13:22, Michael Paquier <michael.paqu...@gmail.com> wrote:

> On Sat, Jun 11, 2016 at 9:44 AM, Thom Brown <t...@linux.com> wrote:
> > It may be the wrong way of going about it, but you get the idea of what
> I'm
> > suggesting we output instead.
>
> Surely things could be better. So +1 to be more verbose here.
>
> +            if (recoveryStopTime == 0)
> +                snprintf(reason, sizeof(reason),
> +                        "recovery reached consistency before recovery
> target time of \"%s\"\n",
> +                        timestamptz_to_str(recoveryTargetTime));
> "Reaching consistency" is not exact for here. I'd rather say "finished
> recovery without reaching target blah"
>

Yeah, sounds fine.


>
> +            if (recoveryStopXid == 0)
> Checking for InvalidTransactionId is better here.
>

Agreed.


> And it would be good to initialize recoveryStopTime and
> recoveryStopXid as those are set only when a recovery target is
> reached.
>

Aren't those already set by recoveryStopsBefore()?

Revised patch attached, with new wording and covering recovery target name
case.

Thom
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e4645a3..4033a2d 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7105,19 +7105,34 @@ StartupXLOG(void)
 		 * timeline changed.
 		 */
 		if (recoveryTarget == RECOVERY_TARGET_XID)
-			snprintf(reason, sizeof(reason),
-					 "%s transaction %u",
-					 recoveryStopAfter ? "after" : "before",
-					 recoveryStopXid);
+			if (recoveryStopXid == InvalidTransactionId)
+				snprintf(reason, sizeof(reason),
+						"recovery finished withoutrecovery target xid of \"%u\"\n",
+						recoveryTargetXid);
+			else
+				snprintf(reason, sizeof(reason),
+						 "%s transaction %u",
+						 recoveryStopAfter ? "after" : "before",
+						 recoveryStopXid);
 		else if (recoveryTarget == RECOVERY_TARGET_TIME)
-			snprintf(reason, sizeof(reason),
-					 "%s %s\n",
-					 recoveryStopAfter ? "after" : "before",
-					 timestamptz_to_str(recoveryStopTime));
+			if (recoveryStopTime == 0)
+				snprintf(reason, sizeof(reason),
+						"recovery finished without recovery target time of \"%s\"\n",
+						timestamptz_to_str(recoveryTargetTime));
+			else
+				snprintf(reason, sizeof(reason),
+						 "%s %s\n",
+						 recoveryStopAfter ? "after" : "before",
+						 timestamptz_to_str(recoveryStopTime));
 		else if (recoveryTarget == RECOVERY_TARGET_NAME)
-			snprintf(reason, sizeof(reason),
-					 "at restore point \"%s\"",
-					 recoveryStopName);
+			if (*recoveryStopName == '\0')
+				snprintf(reason, sizeof(reason),
+						"recovery finished without reaching recovery target name of \"%s\"\n",
+						recoveryTargetName);
+			else
+				snprintf(reason, sizeof(reason),
+						 "at restore point \"%s\"",
+						 recoveryStopName);
 		else if (recoveryTarget == RECOVERY_TARGET_IMMEDIATE)
 			snprintf(reason, sizeof(reason), "reached consistency");
 		else
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to