PITR Error Message assistance

[Simon Riggs]

Could I ask for feedback on the error messages used with the archiver
and restore functionality? Possibly those of you with a hand in the
Error Message style guide?

The messages need the eye of some administrators to suggest some better
phrases. Some probably need some explanation, but I'll leave that for
you to decide which...and what you think it should say instead. Thinking
that if they need explanation, they probably are worded wrongly...

These are copied straight from recent patch:

+ elog(WARNING, "could not set notify for archiver to read log file %u,
segment %u",


+ ereport(LOG,
+ (errmsg("recovery.conf found...starting archive recovery")));

+ elog(LOG, "redo: restored \"%s\" from archive", restoreXlog);

+ elog(LOG, "rename failed %s %s",recoveryXlog, lastrecoXlog);

+ elog(LOG, "redo: cannot restore \"%s\" from archive", restoreXlog);


+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write archive_status file \"%s\"
",
+ tmppath)));

+ elog(LOG, "redo: moving last restored xlog to %s", tmppath);

+ elog(LOG, "redo: rename failed");

+ elog(LOG, "redo: archive chain ends; using local copy of \"%s\"",
restoreXlog);


+ (errmsg("archive recovery complete")));


+ ereport(LOG,
! (errmsg("too many transaction log files, removing \"%s\"",
xlde->d_name)));

+ ereport(WARNING,
+ (errcode_for_file_access(),
+ errmsg("chkpt: cannot find archive_status file: %s ",
+ rlogpath)));


+ elog(WARNING, "chkpt: archiving not yet started for log file
%s",
+ xlog);



DEBUG MESSAGES

+ elog(LOG, "backend: written %s", rlogpath );
+

+ elog(LOG, "postmaster: WAKEN_ARCHIVER received, sending SIGUSR1 to
archiver");

+ elog(LOG, "chkpt: archiving done for log file %s",
+ xlog);

+ elog(LOG, "redo: system(%s)", xlogRestoreCmd);


Thanks,

Best regards, Simon Riggs


---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to ***@postgresql.org)

[Peter Eisentraut]

Post by Simon Riggs
+ elog(WARNING, "could not set notify for archiver to read log file
%u, segment %u",

Reason? (disk full, network down, leap year?)

I think elog() calls don't get translated. You should always use
ereport. Tom would know more about the distinction.

Post by Simon Riggs
+ ereport(LOG,
+ (errmsg("recovery.conf found...starting archive recovery")));

comma

Post by Simon Riggs
+ elog(LOG, "redo: cannot restore \"%s\" from archive",
restoreXlog);

Reason?

Post by Simon Riggs
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not write archive_status file
\"%s\" ",
+ tmppath)));

Reason?

Post by Simon Riggs
+ elog(LOG, "redo: moving last restored xlog to %s", tmppath);

Probably a user doesn't know what "xlog" is. Also, I don't like the
prefix: style you use. Just tell what happened.

Post by Simon Riggs
+ elog(LOG, "redo: rename failed");

We don't write "foo failed", but "could not do foo". And again: reason?

Post by Simon Riggs
+ elog(LOG, "redo: archive chain ends; using local copy of \"%s\"",
restoreXlog);
+ ereport(LOG,
! (errmsg("too many transaction log files, removing \"%s\"",
xlde->d_name)));

How/where is the limit defined?

Post by Simon Riggs
+ ereport(WARNING,
+ (errcode_for_file_access(),
+ errmsg("chkpt: cannot find archive_status file: %s ",
+ rlogpath)));

There is enough space that you can write "checkpoint". Or actually
don't write anything. What is the archive_status file?

Post by Simon Riggs
+ elog(WARNING, "chkpt: archiving not yet started for log
file %s",
+ xlog);

What does that tell me?

Post by Simon Riggs
DEBUG MESSAGES

Debug messages should have a DEBUG severity.

Post by Simon Riggs
+ elog(LOG, "backend: written %s", rlogpath );
+
+ elog(LOG, "postmaster: WAKEN_ARCHIVER received, sending SIGUSR1
to archiver");
+ elog(LOG, "chkpt: archiving done for log file %s",
+ xlog);
+ elog(LOG, "redo: system(%s)", xlogRestoreCmd);

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

[Tom Lane]

Post by Peter Eisentraut

Post by Simon Riggs
+ elog(WARNING, "could not set notify for archiver to read log file
%u, segment %u",

Reason? (disk full, network down, leap year?)
I think elog() calls don't get translated. You should always use
ereport. Tom would know more about the distinction.

elog is deprecated except for debugging or "can't-happen" messages.
Anything user-facing ought to be reported with ereport. In this case
elog might be okay --- it's not clear to me from this snippet whether
the condition is one a user would be likely to see.

There's plenty of detail about all this in chapter 45 of the docs:
http://www.postgresql.org/docs/7.4/static/source.html
and I think most of Peter's comments trace directly to items in the
message style guide there.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster

[Simon Riggs]

Thanks very much for your comments.

Post by Tom Lane

Post by Peter Eisentraut

Post by Simon Riggs
+ elog(WARNING, "could not set notify for archiver to read log file
%u, segment %u",

Reason? (disk full, network down, leap year?)

? "could not increment year number (additional day found)" :)

Post by Tom Lane

Post by Peter Eisentraut
I think elog() calls don't get translated. You should always use
ereport. Tom would know more about the distinction.

elog is deprecated except for debugging or "can't-happen" messages.
Anything user-facing ought to be reported with ereport. In this case
elog might be okay --- it's not clear to me from this snippet whether
the condition is one a user would be likely to see.

Thanks for clarifying that, I wasn't clear on that. I'll go through and
make any required changes.

Post by Tom Lane
http://www.postgresql.org/docs/7.4/static/source.html
and I think most of Peter's comments trace directly to items in the
message style guide there.

Yes, I've read that and this post was all about discussing this and
applying it as part of polishing work.

I was really looking for some suggestions rather than a critique - the
messages were not exactly the bit I was focusing on in dev. Peter has
highlighted the extent of improvement, so I'll get on it now.

(For later, I've found it confusing that the Developer's FAQ makes no
mention of those notes, nor the other way around - I'll submit some
suggested changes to make everything clearer for those on their first
patch....and yes, I've tried hard to RTFM)

Can I just clarify whether the end of June freeze does or does not apply
to documentation? Clearly, I have a significant amount of documentation
to write also, which presumably will need review also.

What are the plans for review? I've not really heard much as yet...

Best Regards, Simon Riggs


---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

[Tom Lane]

Post by Simon Riggs
Can I just clarify whether the end of June freeze does or does not apply
to documentation?

Absolutely not. Usually we're still working on the docs during beta.
Note also that this is *feature* freeze not code freeze. Bug fixes and
loose ends are not a problem for later.

Post by Simon Riggs
What are the plans for review?

Right at the moment I'm hip-deep in nested transactions ... I hope to
commit that today and then start looking at your stuff.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to ***@postgresql.org