Обсуждение: idle-in-transaction timeout error does not give a hint
idle-in-transaction timeout error closed the session. I think in this case the error message should give a hint something like other errors (for example ERRCODE_CRASH_SHUTDOWN or ERRCODE_T_R_SERIALIZATION_FAILURE) to ask users to reconnect. Attached patch does that. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index a3b9757565..c3e4380603 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3138,7 +3138,9 @@ ProcessInterrupts(void) if (IdleInTransactionSessionTimeout > 0) ereport(FATAL, (errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT), - errmsg("terminating connection due to idle-in-transaction timeout"))); + errmsg("terminating connection due to idle-in-transaction timeout"), + errhint("In a moment you should be able to reconnect to the" + " database and repeat your command."))); else IdleInTransactionSessionTimeoutPending = false;
>From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp]
>Sent: Wednesday, November 28, 2018 12:18 PM
>To: pgsql-hackers@lists.postgresql.org
>Subject: idle-in-transaction timeout error does not give a hint
>
>idle-in-transaction timeout error closed the session. I think in this case the error
>message should give a hint something like other errors (for example
>ERRCODE_CRASH_SHUTDOWN or
>ERRCODE_T_R_SERIALIZATION_FAILURE) to ask users to reconnect.
>Attached patch does that.
Hi, it makes sense to me. One can submit transaction again same as other cases
you mentioned.
I didn't attach the patch but according to my simple experiment
in psql the output would become the following:
FATAL: terminating connection due to idle-in-transaction timeout
HINT: In a moment you should be able to reconnect to the
database and repeat your command.
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
Regards,
Takeshi Ideriha
> Hi, it makes sense to me. One can submit transaction again same as other cases
> you mentioned.
>
> I didn't attach the patch but according to my simple experiment
> in psql the output would become the following:
>
> FATAL: terminating connection due to idle-in-transaction timeout
> HINT: In a moment you should be able to reconnect to the
> database and repeat your command.
Alternative HINT message would be something like:
HINT: In a moment you should be able to reconnect to the
database and restart your transaction.
This could make the meaning of the error (transaction aborted)
cleaner and might give a better suggestion to the user.
Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp
>> Hi, it makes sense to me. One can submit transaction again same as >> other cases you mentioned. >> >> I didn't attach the patch but according to my simple experiment in >> psql the output would become the following: >> >> FATAL: terminating connection due to idle-in-transaction timeout >> HINT: In a moment you should be able to reconnect to the >> database and repeat your command. > >Alternative HINT message would be something like: > >HINT: In a moment you should be able to reconnect to the > database and restart your transaction. > >This could make the meaning of the error (transaction aborted) cleaner and might give >a better suggestion to the user. Agreed. Changing "command" to "transaction" seems more accurate. People might think only the command they hit is not sent but transaction is still alive though it's of course unnatural that transaction is alive after connection is terminated. In this case you could change the comment issued by other errors mentioned while you're at it. Regards, Takeshi Ideriha
>>> Hi, it makes sense to me. One can submit transaction again same as >>> other cases you mentioned. >>> >>> I didn't attach the patch but according to my simple experiment in >>> psql the output would become the following: >>> >>> FATAL: terminating connection due to idle-in-transaction timeout >>> HINT: In a moment you should be able to reconnect to the >>> database and repeat your command. >> >>Alternative HINT message would be something like: >> >>HINT: In a moment you should be able to reconnect to the >> database and restart your transaction. >> >>This could make the meaning of the error (transaction aborted) cleaner and might give >>a better suggestion to the user. > > Agreed. Changing "command" to "transaction" seems more accurate. People might think > only the command they hit is not sent but transaction is still alive though it's of course unnatural > that transaction is alive after connection is terminated. > > In this case you could change the comment issued by other errors mentioned while you're at it. > > Regards, > Takeshi Ideriha I have added this to the next CF (2019-01). Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Hi Ideriha-san, >>>> Hi, it makes sense to me. One can submit transaction again same as >>>> other cases you mentioned. >>>> >>>> I didn't attach the patch but according to my simple experiment in >>>> psql the output would become the following: >>>> >>>> FATAL: terminating connection due to idle-in-transaction timeout >>>> HINT: In a moment you should be able to reconnect to the >>>> database and repeat your command. >>> >>>Alternative HINT message would be something like: >>> >>>HINT: In a moment you should be able to reconnect to the >>> database and restart your transaction. >>> >>>This could make the meaning of the error (transaction aborted) cleaner and might give >>>a better suggestion to the user. >> >> Agreed. Changing "command" to "transaction" seems more accurate. People might think >> only the command they hit is not sent but transaction is still alive though it's of course unnatural >> that transaction is alive after connection is terminated. >> >> In this case you could change the comment issued by other errors mentioned while you're at it. >> >> Regards, >> Takeshi Ideriha > > I have added this to the next CF (2019-01). Please find attached patch which addresses the point above. BTW, would you like to be added to the CF item as a reviewer? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 9a948f825d..27337a21da 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3150,7 +3150,9 @@ ProcessInterrupts(void) if (IdleInTransactionSessionTimeout > 0) ereport(FATAL, (errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT), - errmsg("terminating connection due to idle-in-transaction timeout"))); + errmsg("terminating connection due to idle-in-transaction timeout"), + errhint("In a moment you should be able to reconnect to the" + " database and restart your transaction."))); else IdleInTransactionSessionTimeoutPending = false;
Hello. At Thu, 29 Nov 2018 09:16:01 +0900 (JST), Tatsuo Ishii <ishii@sraoss.co.jp> wrote in <20181129.091601.830026250066724330.t-ishii@sraoss.co.jp> > >>> Hi, it makes sense to me. One can submit transaction again same as > >>> other cases you mentioned. > >>> > >>> I didn't attach the patch but according to my simple experiment in > >>> psql the output would become the following: > >>> > >>> FATAL: terminating connection due to idle-in-transaction timeout > >>> HINT: In a moment you should be able to reconnect to the > >>> database and repeat your command. > >> > >>Alternative HINT message would be something like: > >> > >>HINT: In a moment you should be able to reconnect to the > >> database and restart your transaction. > >> > >>This could make the meaning of the error (transaction aborted) cleaner and might give > >>a better suggestion to the user. > > > > Agreed. Changing "command" to "transaction" seems more accurate. People might think > > only the command they hit is not sent but transaction is still alive though it's of course unnatural > > that transaction is alive after connection is terminated. "session" seems more correct and covers both (and the other instances below). > > In this case you could change the comment issued by other errors mentioned while you're at it. > > > > Regards, > > Takeshi Ideriha > > I have added this to the next CF (2019-01). I found several more instances. "terminating connection due to conflict with recovery" (3 places) "terminating connection due to administrator command" (1 place) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Hi, > Hello. > > At Thu, 29 Nov 2018 09:16:01 +0900 (JST), Tatsuo Ishii <ishii@sraoss.co.jp> wrote in <20181129.091601.830026250066724330.t-ishii@sraoss.co.jp> >> >>> Hi, it makes sense to me. One can submit transaction again same as >> >>> other cases you mentioned. >> >>> >> >>> I didn't attach the patch but according to my simple experiment in >> >>> psql the output would become the following: >> >>> >> >>> FATAL: terminating connection due to idle-in-transaction timeout >> >>> HINT: In a moment you should be able to reconnect to the >> >>> database and repeat your command. >> >> >> >>Alternative HINT message would be something like: >> >> >> >>HINT: In a moment you should be able to reconnect to the >> >> database and restart your transaction. >> >> >> >>This could make the meaning of the error (transaction aborted) cleaner and might give >> >>a better suggestion to the user. >> > >> > Agreed. Changing "command" to "transaction" seems more accurate. People might think >> > only the command they hit is not sent but transaction is still alive though it's of course unnatural >> > that transaction is alive after connection is terminated. > > "session" seems more correct and covers both (and the other > instances below). But "reconnect to the database" already implies "restarting session", no? If so, "...restart your session" would sound redundant to me. A native English speaker's comment is welcome. I may be wrong. >> > In this case you could change the comment issued by other errors mentioned while you're at it. >> > >> > Regards, >> > Takeshi Ideriha >> >> I have added this to the next CF (2019-01). > > I found several more instances. > > "terminating connection due to conflict with recovery" (3 places) > "terminating connection due to administrator command" (1 place) > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center >
>From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] > >Hi Ideriha-san, > >>>>> Hi, it makes sense to me. One can submit transaction again same as >>>>> other cases you mentioned. >>>>> >>>>> I didn't attach the patch but according to my simple experiment in >>>>> psql the output would become the following: >>>>> >>>>> FATAL: terminating connection due to idle-in-transaction timeout >>>>> HINT: In a moment you should be able to reconnect to the >>>>> database and repeat your command. >>>> >>>>Alternative HINT message would be something like: >>>> >>>>HINT: In a moment you should be able to reconnect to the >>>> database and restart your transaction. >>>> >>>>This could make the meaning of the error (transaction aborted) >>>>cleaner and might give a better suggestion to the user. >>> >>> Agreed. Changing "command" to "transaction" seems more accurate. >>> People might think only the command they hit is not sent but >>> transaction is still alive though it's of course unnatural that transaction is alive after >connection is terminated. >Please find attached patch which addresses the point above. Thank you for the update. It looks good to me on this point. Are you planning to change other similar messages? >BTW, would you like to be added to the CF item as a reviewer? Yeah, I've already added myself as a review. Regards, Takeshi Ideriha
>>>>>Alternative HINT message would be something like: >>>>> >>>>>HINT: In a moment you should be able to reconnect to the >>>>> database and restart your transaction. >>>>> >>>>>This could make the meaning of the error (transaction aborted) >>>>>cleaner and might give a better suggestion to the user. >>>> >>>> Agreed. Changing "command" to "transaction" seems more accurate. >>>> People might think only the command they hit is not sent but >>>> transaction is still alive though it's of course unnatural that transaction is alive after >>connection is terminated. > > >>Please find attached patch which addresses the point above. > Thank you for the update. It looks good to me on this point. > Are you planning to change other similar messages? No, because this is the only message related to an explicit transaction. Other messages are not related to explicit transactions, so current messages look ok to me. >>BTW, would you like to be added to the CF item as a reviewer? > Yeah, I've already added myself as a review. Thanks. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
>From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] >Subject: Re: idle-in-transaction timeout error does not give a hint > >>>>>>Alternative HINT message would be something like: >>>>>> >>>>>>HINT: In a moment you should be able to reconnect to the >>>>>> database and restart your transaction. >>>>>> >>>>>>This could make the meaning of the error (transaction aborted) >>>>>>cleaner and might give a better suggestion to the user. >>>>> >>>>> Agreed. Changing "command" to "transaction" seems more accurate. >>>>> People might think only the command they hit is not sent but >>>>> transaction is still alive though it's of course unnatural that >>>>> transaction is alive after >>>connection is terminated. >> >> >>>Please find attached patch which addresses the point above. >> Thank you for the update. It looks good to me on this point. >> Are you planning to change other similar messages? > >No, because this is the only message related to an explicit transaction. Other >messages are not related to explicit transactions, so current messages look ok to me. Sure. I didn't have a strong opinion about it, so it's ok. Regards, Takeshi Ideriha
On Tue, Dec 04, 2018 at 04:07:34AM +0000, Ideriha, Takeshi wrote: > Sure. I didn't have a strong opinion about it, so it's ok. From what I can see this is waiting input from a native English speaker, so for now I have moved this entry to next CF. -- Michael
Вложения
Hi,
On Monday, February 4, 2019 2:15 AM +0000, Michael Paquier wrote:
> On Tue, Dec 04, 2018 at 04:07:34AM +0000, Ideriha, Takeshi wrote:
> > Sure. I didn't have a strong opinion about it, so it's ok.
> From what I can see this is waiting input from a native English speaker, so for now I have moved this entry to next
CF.
Although I also use English in daily life,
I am not from a native English-speaking country.
But I'm giving it a try, since the current patch is not that complex to check.
> HINT: In a moment you should be able to reconnect to the
> database and restart your transaction.
I think this error hint message makes sense for idle-in-transaction timeout.
It's grammatically correct and gives a clearer hint for that.
I agree that "reconnect to database" implies "restart session",
so it may not be the best word for errhint message.
Now the next point raised by Ideriha-san is whether the
other places in the code should also be changed.
I also checked the source code where the other errhints appear,
besides idle-in-transaction.
(ERRCODE_CRASH_SHUTDOWN and ERRCODE_T_R_SERIALIZATION_FAILURE)
Those errcodes use "repeat your command" for the hint.
> errhint("In a moment you should be able to reconnect to the"
> " database and repeat your command.")));
(1) (errcode(ERRCODE_CRASH_SHUTDOWN)
As per its errdetail, the transaction is rollbacked,
so "restart transaction" also makes sense.
(2) ERRCODE_T_R_SERIALIZATION_FAILURE
The second one is under the clause below...
> if (RecoveryConflictPending && DoingCommandRead)
...so using "repeat your command" makes sense.
I'm fine with retaining the other two hint messages as they are.
Regardless if we want to change the other similarly written
errhint messages or add errhint messages for other errcodes
that don't have it, I think the latest patch that provides errhint
message for idle-in-transaction timeout may be changed to
"Ready for Committer", as it still applies and the tests
successfully passed.
Regards,
Kirk Jamison
On 2018-11-28 04:17, Tatsuo Ishii wrote:
> + errmsg("terminating connection due to idle-in-transaction timeout"),
> + errhint("In a moment you should be able to reconnect to the"
> + " database and repeat your command.")));
Personally, I don't find this hint particularly necessary. The session
was terminated because nothing was happening, so the real fix on the
application side is probably more involved than just retrying. This is
different from some of the other cases that were cited, such as
serialization conflicts, where you just got unlucky due to concurrent
events. In the case of idle-in-transaction-timeout, the fault is
entirely your own.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> Personally, I don't find this hint particularly necessary. The session > was terminated because nothing was happening, so the real fix on the > application side is probably more involved than just retrying. This is > different from some of the other cases that were cited, such as > serialization conflicts, where you just got unlucky due to concurrent > events. In the case of idle-in-transaction-timeout, the fault is > entirely your own. Hum. Sounds like a fair argument. Ideriha-san, what do you think? Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
>From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] > >> Personally, I don't find this hint particularly necessary. The >> session was terminated because nothing was happening, so the real fix >> on the application side is probably more involved than just retrying. >> This is different from some of the other cases that were cited, such >> as serialization conflicts, where you just got unlucky due to >> concurrent events. In the case of idle-in-transaction-timeout, the >> fault is entirely your own. > >Hum. Sounds like a fair argument. Ideriha-san, what do you think? > Hi. As Peter mentioned, application code generally needs to handle more things than retrying. So I'm ok with not adding this hint. Regards, Takeshi Ideriha
>>From: Tatsuo Ishii [mailto:ishii@sraoss.co.jp] >> >>> Personally, I don't find this hint particularly necessary. The >>> session was terminated because nothing was happening, so the real fix >>> on the application side is probably more involved than just retrying. >>> This is different from some of the other cases that were cited, such >>> as serialization conflicts, where you just got unlucky due to >>> concurrent events. In the case of idle-in-transaction-timeout, the >>> fault is entirely your own. >> >>Hum. Sounds like a fair argument. Ideriha-san, what do you think? >> > > Hi. > As Peter mentioned, application code generally needs to handle more things > than retrying. > So I'm ok with not adding this hint. I have changed the patch status to "Withdrawn". Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp