Обсуждение: PITR Error Message assistance
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
Simon Riggs wrote:
> + 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.
> + ereport(LOG,
> + (errmsg("recovery.conf found...starting archive recovery")));
comma
> + elog(LOG, "redo: cannot restore \"%s\" from archive",
> restoreXlog);
Reason?
> + ereport(ERROR,
> + (errcode_for_file_access(),
> + errmsg("could not write archive_status file
> \"%s\" ",
> + tmppath)));
Reason?
> + 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.
> + elog(LOG, "redo: rename failed");
We don't write "foo failed", but "could not do foo". And again: reason?
> + 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?
> + 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?
> + elog(WARNING, "chkpt: archiving not yet started for log
> file %s",
> + xlog);
What does that tell me?
> DEBUG MESSAGES
Debug messages should have a DEBUG severity.
>
> + 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);
Peter Eisentraut <peter_e@gmx.net> writes: > Simon Riggs wrote: >> + 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
On Wed, 2004-06-30 at 15:58, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: Thanks very much for your comments. > > Simon Riggs wrote: > >> + 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)" :) > > > 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. > 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. 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
Simon Riggs <simon@2ndquadrant.com> writes:
> 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.
> 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