Re: Named restore points

On Wed, Feb 9, 2011 at 4:53 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> Committed. Thanks for the patch and the review.

- * We also track the timestamp of the latest applied COMMIT/ABORT record
- * in XLogCtl->recoveryLastXTime, for logging purposes.
+ * We also track the timestamp of the latest applied COMMIT/ABORT/RESTORE POINT
+ * record in XLogCtl->recoveryLastXTime, for logging purposes.

Tracking the timestamp of the restore point record in recoveryLastXTime
messes up pg_last_xact_replay_timestamp which uses recoveryLastXTime.
The timestamp of the restore point is wrongly returned as that of the latest
transaction, by the function.

As far as I read the patch, I don't think that it's necessary to track the
timestamp of the restore point. The attached patch changes the code so
that it doesn't track the timestamp of the restore point.

+    if (strlen(restore_name_str) >= MAXFNAMELEN)
+        ereport(ERROR,
+                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                 errmsg("value too long for restore point")));

I think that logging the maximum length of the name is useful as follows:

    ERROR:  value too long for restore point (max 63 characters)

So the attached patch also changes the log message that way.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center