Thread: SSI error messages
Some of these new error messages from the SSI code are a mouthful: not enough elements in RWConflictPool to record a rw-conflict not enough elements in RWConflictPool to record a potentialrw-conflict These are basically "out of shared memory" conditions that could be moved to a DETAIL message. Canceled on identification as a pivot, during conflict out checking. Canceled on conflict out to old pivot %u. Canceledon identification as a pivot, with conflict out to old committed transaction %u. Canceled on conflict out to oldpivot. Canceled on identification as a pivot, during conflict in checking. Canceled on identification as a pivot,during write. Canceled on conflict out to pivot %u, during read. Canceled on identification as a pivot, duringcommit attempt. Canceled on commit attempt with conflict in from prepared pivot. These are DETAIL messages, with the rest of the report saying ERROR: could not serialize access due to read/write dependencies among transactions HINT: The transaction might succeedif retried. AFAICT, the documentation doesn't mention anything about this "pivot" that keeps coming up. Is it useful that have the user face this information? Is there anything a user can derive from seeing one of these messages versus another, and in addition to the error and hint, that would help them address the situation?
Peter Eisentraut <peter_e@gmx.net> wrote: > Some of these new error messages from the SSI code are a mouthful: > > not enough elements in RWConflictPool to record a rw-conflict > not enough elements in RWConflictPool to record a potential > rw-conflict > > These are basically "out of shared memory" conditions that could > be moved to a DETAIL message. Yes and no. The resource which is exhausted at this point *is* in shared memory, but its allocation of space is fixed at startup -- it's not competing with other users of shared memory for the shared memory "slush space". I have some concern that an "out of shared memory" message might confuse people on that aspect of the issue. That said, if people find it helps more than it hurts, I have no objection to a wording change. > Canceled on identification as a pivot, during conflict out > checking. > Canceled on conflict out to old pivot %u. > Canceled on identification as a pivot, with conflict out to > old committed transaction %u. > Canceled on conflict out to old pivot. > Canceled on identification as a pivot, during conflict in > checking. > Canceled on identification as a pivot, during write. > Canceled on conflict out to pivot %u, during read. > Canceled on identification as a pivot, during commit attempt. > Canceled on commit attempt with conflict in from prepared > pivot. > > These are DETAIL messages, with the rest of the report saying > > ERROR: could not serialize access due to read/write > dependencies among transactions > HINT: The transaction might succeed if retried. > > AFAICT, the documentation doesn't mention anything about this > "pivot" that keeps coming up. Good point. It's in the README-SSI and the Wiki page, but not in the user docs; so using it in the error detail seems likely to confuse. Also, some of the information is referring to phases of processing which can only be understood by looking at the source code, like "during conflict out checking." While it might not be entirely unreasonable to document what a pivot is, documenting some of the other language is really out of the question. > Is it useful that have the user face this information? Is there > anything a user can derive from seeing one of these messages > versus another, and in addition to the error and hint, that would > help them address the situation? A user with a firm grasp of SSI might be better able to figure out mitigation techniques for frequently occurring rollback scenarios with that detail. We did have one thread where someone was hoping for as much information as we could provide, but I don't know how typical that will be: http://archives.postgresql.org/pgsql-hackers/2010-09/msg01133.php I find the differentiation of causes helpful when working through test cases, but moving this information to DEBUG messages might satisfy that need. I've also wondered whether that HINT line is worthwhile. I have a suspicion that we might sometimes find the information conveyed by the detail useful when responding to users with questions; but the language as it stands seems confusing for users. -Kevin
On Jul 15, 2011, at 12:06 PM, "Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote: > I have a suspicion that we might sometimes find the information > conveyed by the detail useful when responding to users with > questions; but the language as it stands seems confusing for users. I think removing info here or making it harder to get would be a mistake. SSI is a complicated technology and we are likelyto find that we need more debugging and instrumentation, not less. ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> On Jul 15, 2011, at 12:06 PM, "Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote:
>> I have a suspicion that we might sometimes find the information
>> conveyed by the detail useful when responding to users with
>> questions; but the language as it stands seems confusing for users.
> I think removing info here or making it harder to get would be a mistake.
Agreed. If we can clarify the messages, and/or make sure the
terminology they use is documented, that'd be a good thing; but let's
not just remove them.
I think that Peter's real concern is whether these are worth
translating, and I share that doubt. Perhaps we should invent an
errdetail_internal, parallel to errmsg_internal, that works like
errdetail but doesn't treat the message as a candidate for translation?
regards, tom lane
Excerpts from Tom Lane's message of vie jul 15 14:33:34 -0400 2011: > I think that Peter's real concern is whether these are worth > translating, and I share that doubt. Perhaps we should invent an > errdetail_internal, parallel to errmsg_internal, that works like > errdetail but doesn't treat the message as a candidate for translation? Yeah. The other point is that translated versions of those phrases are likely to introduce randomly diverging terms, which is not going to help with debugging. As long as the technology is new enough that a user is going to need help from -hackers (or at least someone who read and grokked README.SSI) to debug a problem, there's no benefit to translating those errdetails. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Tom Lane's message: > >> I think that Peter's real concern is whether these are worth >> translating, and I share that doubt. Perhaps we should invent an >> errdetail_internal, parallel to errmsg_internal, that works like >> errdetail but doesn't treat the message as a candidate for >> translation? > > Yeah. The other point is that translated versions of those > phrases are likely to introduce randomly diverging terms, which is > not going to help with debugging. As long as the technology is > new enough that a user is going to need help from -hackers (or at > least someone who read and grokked README.SSI) to debug a problem, > there's no benefit to translating those errdetails. OK, since that seems to be the consensus, I put a patch together. Testing it now. Will post once I've confirmed it doesn't break anything. -Kevin
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> wrote: > Alvaro Herrera <alvherre@commandprompt.com> wrote: >> Excerpts from Tom Lane's message: >> >>> I think that Peter's real concern is whether these are worth >>> translating, and I share that doubt. Perhaps we should invent >>> an errdetail_internal, parallel to errmsg_internal, that works >>> like errdetail but doesn't treat the message as a candidate for >>> translation? >> >> Yeah. The other point is that translated versions of those >> phrases are likely to introduce randomly diverging terms, which >> is not going to help with debugging. As long as the technology >> is new enough that a user is going to need help from -hackers (or >> at least someone who read and grokked README.SSI) to debug a >> problem, there's no benefit to translating those errdetails. > > OK, since that seems to be the consensus, I put a patch together. > Testing it now. Will post once I've confirmed it doesn't break > anything. OK, after getting distracted by test failures caused by an unrelated commit, I've confirmed that this passes my usual tests. I don't know anything about the tools used for extracting the text for the translators, so if that needs any corresponding adjustment, someone will need to point me in the right direction or cover that part separately. The leading whitespace changes in predicate.c are from pgindent. -Kevin
Attachment
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> OK, after getting distracted by test failures caused by an unrelated
> commit, I've confirmed that this passes my usual tests. I don't
> know anything about the tools used for extracting the text for the
> translators, so if that needs any corresponding adjustment, someone
> will need to point me in the right direction or cover that part
> separately.
Well, the point is that this function *isn't* going to be known to the
NLS code, so AFAICS no adjustments should be needed there. You did miss
some places that ought to be updated (mumble sources.sgml mumble) but
unless I hear objections to the whole idea, I'll fix and apply this
tomorrow.
regards, tom lane
Tom Lane wrote: > "Kevin Grittner" writes: >> OK, after getting distracted by test failures caused by an >> unrelated commit, I've confirmed that this passes my usual tests. >> I don't know anything about the tools used for extracting the text >> for the translators, so if that needs any corresponding >> adjustment, someone will need to point me in the right direction >> or cover that part separately. > > Well, the point is that this function *isn't* going to be known to > the NLS code, so AFAICS no adjustments should be needed there. That seemed likely, but not knowing the tools, I wasn't sure. > You did miss some places that ought to be updated (mumble > sources.sgml mumble) Sorry I missed that; sources.sgml covered with the attached. -Kevin
Attachment
"Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
> Tom Lane wrote:
>> You did miss some places that ought to be updated (mumble
>> sources.sgml mumble)
> Sorry I missed that; sources.sgml covered with the attached.
Oh, I'd already fixed that locally, but thanks. Patch is committed now.
regards, tom lane
On 16.07.2011 03:14, Tom Lane wrote: > "Kevin Grittner"<Kevin.Grittner@wicourts.gov> writes: >> OK, after getting distracted by test failures caused by an unrelated >> commit, I've confirmed that this passes my usual tests. I don't >> know anything about the tools used for extracting the text for the >> translators, so if that needs any corresponding adjustment, someone >> will need to point me in the right direction or cover that part >> separately. > > Well, the point is that this function *isn't* going to be known to the > NLS code, so AFAICS no adjustments should be needed there. You did miss > some places that ought to be updated (mumble sources.sgml mumble) but > unless I hear objections to the whole idea, I'll fix and apply this > tomorrow. I find it strange to simply leave those strings untranslated. It's going to look wrong, like someone just forgot to translate them. However, I agree it's perhaps a bit too much detail to translate all of those messages, and the translations would probably sound weird because there isn't established terms for these things yet. I think I would prefer something like this: ERROR: could not serialize access due to read/write dependencies among transactions DETAIL: Reason code: %s HINT: The transaction might succeed if retried. Where %s gets the current detail field, untranslated, like: Canceled on commit attempt with conflict in from prepared pivot. Or perhaps shorten that to just "conflict in from prepared pivot", as the fact that it happened on commit attempt should be clear from the context - the error happened at a COMMIT statement. That would be similar to what we do with OS error messages, with %m. It would be more obvious that the untranslated message is some internal information that the user is not expect to understand, and that it is untranslated on purpose. That's my 2c, anyway. I see you committed this already, I don't violently object to that either... -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> I think I would prefer something like this:
> ERROR: could not serialize access due to read/write dependencies among
> transactions
> DETAIL: Reason code: %s
> HINT: The transaction might succeed if retried.
> That's my 2c, anyway. I see you committed this already, I don't
> violently object to that either...
Well, as I mentioned in the commit message, I've thought for some time
that there were use cases for errdetail_internal. Whether these
particular places in predicate.c use it or not doesn't affect that.
I don't have a strong opinion about whether to do it like you suggest,
other than that the proposed wording doesn't meet the message style
guideline about detail messages being complete sentences.
regards, tom lane
On lör, 2011-07-16 at 15:01 -0400, Tom Lane wrote: > Well, as I mentioned in the commit message, I've thought for some time > that there were use cases for errdetail_internal. Whether these > particular places in predicate.c use it or not doesn't affect that. Looking at commit 1af37ec96d97722aeb527f5f43d6f6f2304f0861, not all of these are strings that don't need to be translated. For example, you can't assume that the translation of "%s: %s" is "%s: %s" or that the translation of "%s (%x)" is "%s (%x)". I'll review those in detail when I find some time, but some of them will probably have to be reverted.
On lör, 2011-07-16 at 21:55 +0300, Heikki Linnakangas wrote: > I find it strange to simply leave those strings untranslated. It's > going > to look wrong, like someone just forgot to translate them. However, I > agree it's perhaps a bit too much detail to translate all of those > messages, and the translations would probably sound weird because > there > isn't established terms for these things yet. > > I think I would prefer something like this: > > ERROR: could not serialize access due to read/write dependencies > among > transactions > DETAIL: Reason code: %s > HINT: The transaction might succeed if retried. Yes, I think that would be better. I will work on that.
Excerpts from Peter Eisentraut's message of mié jul 27 16:19:22 -0400 2011: > On lör, 2011-07-16 at 15:01 -0400, Tom Lane wrote: > > Well, as I mentioned in the commit message, I've thought for some time > > that there were use cases for errdetail_internal. Whether these > > particular places in predicate.c use it or not doesn't affect that. > > Looking at commit 1af37ec96d97722aeb527f5f43d6f6f2304f0861, not all of > these are strings that don't need to be translated. For example, you > can't assume that the translation of "%s: %s" is "%s: %s" or that the > translation of "%s (%x)" is "%s (%x)". I'll review those in detail when > I find some time, but some of them will probably have to be reverted. Hmm, if "%s: %s" has different uses in different places, we're going to need a more involved solution because they might not translate identically. In any case, we need /* translator: */ comments to explain what's going on. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On lör, 2011-07-16 at 21:55 +0300, Heikki Linnakangas wrote:
> I think I would prefer something like this:
>
> ERROR: could not serialize access due to read/write dependencies
> among
> transactions
> DETAIL: Reason code: %s
> HINT: The transaction might succeed if retried.
>
> Where %s gets the current detail field, untranslated, like:
>
> Canceled on commit attempt with conflict in from prepared pivot.
Do you have an idea how to address this case:
@@ -3865,7 +3865,7 @@ CheckForSerializableConflictOut(bool visible, Relation relation, ereport(ERROR,
(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg("could not serialize access
dueto read/write dependencies among transactions"),
- errdetail_internal("Canceled on conflict out to old pivot %u.", xid),
+ errdetail("Reason code: %s.", "canceled on conflict out to old pivot %u", xid), // XXX bogus
errhint("The transaction might succeed if retried."))); if
(SxactHasSummaryConflictIn(MySerializableXact)
Excerpts from Peter Eisentraut's message of vie jul 29 14:46:20 -0400 2011:
> On lör, 2011-07-16 at 21:55 +0300, Heikki Linnakangas wrote:
> > I think I would prefer something like this:
> >
> > ERROR: could not serialize access due to read/write dependencies
> > among
> > transactions
> > DETAIL: Reason code: %s
> > HINT: The transaction might succeed if retried.
> >
> > Where %s gets the current detail field, untranslated, like:
> >
> > Canceled on commit attempt with conflict in from prepared pivot.
>
> Do you have an idea how to address this case:
Call sprintf to expand the %u before ereport()?
> @@ -3865,7 +3865,7 @@ CheckForSerializableConflictOut(bool visible, Relation relation,
> ereport(ERROR,
> (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
> errmsg("could not serialize access due to read/write dependencies among transactions"),
> - errdetail_internal("Canceled on conflict out to old pivot %u.", xid),
> + errdetail("Reason code: %s.", "canceled on conflict out to old pivot %u", xid), // XXX
bogus
> errhint("The transaction might succeed if retried.")));
--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Peter Eisentraut's message of vie jul 29 14:46:20 -0400 2011:
>> On lör, 2011-07-16 at 21:55 +0300, Heikki Linnakangas wrote:
>> Do you have an idea how to address this case:
> Call sprintf to expand the %u before ereport()?
That sounds like way too much work for a wording "improvement" that is
exceedingly marginal in the first place.
IMO the only thing really wrong with those messages is that they aren't
complete sentences, which is a style guideline that we ought to try to
adhere to even if we're not bothering to translate them. Perhaps
"Transaction canceled because of ..." would read better than "Canceled
on ..."?
regards, tom lane
On lör, 2011-07-16 at 21:55 +0300, Heikki Linnakangas wrote: > I think I would prefer something like this: > > ERROR: could not serialize access due to read/write dependencies > among > transactions > DETAIL: Reason code: %s > HINT: The transaction might succeed if retried. I've done it this way now, except that "Reason code" is not marked for translation, for the implementation reasons discussed downthread. I'm looking forward to see how many of these we'll see in practice and how users will react to them.