Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 9:26 PM, Magnus Hagander mag...@hagander.net wrote: On Thu, May 3, 2012 at 7:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: Heh - we already used ERRCODE_CONNECTION_FAILURE on the errors in copy.c. Since COPY can only happen when there is a transaction (right?), I just changed those error messages for consistency. Agreed on changing the message texts to match, but I wonder whether we ought not switch all those SQLSTATEs to something different. Per my comment to Kevin, I think the whole 08 class is meant to be issued on the client side. Maybe it's okay to conflate a server-detected connection loss with client-detected loss, but I'm not convinced. Sure,that's a simple search and replace of course... If we can come to a decision about what codes to actually use. I'm not sure I have much input other than that I agree they need to be different :-) Any further suggestoins for which codes to use? If not, I think I'm going to commit the patch as I had it, because it's not any worse than what we had before (but fixes the annoying messages), and we can always revisit the actual errorcodes later. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: Any further suggestoins for which codes to use? If not, I think I'm going to commit the patch as I had it, because it's not any worse than what we had before (but fixes the annoying messages), and we can always revisit the actual errorcodes later. I'm still a bit uncomfortable about using the 08 codes on the backend side; but on reflection it's hard to see how it could cause any real confusion, so maybe we should just go with that. Another point is that the patch would be shorter and more reliable if you just forced whereToSendOutput = DestNone, without trying to save and restore it. Once the connection is known busted, there is no point in sending any future I/O towards the client, either. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Mon, May 7, 2012 at 5:15 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: Any further suggestoins for which codes to use? If not, I think I'm going to commit the patch as I had it, because it's not any worse than what we had before (but fixes the annoying messages), and we can always revisit the actual errorcodes later. I'm still a bit uncomfortable about using the 08 codes on the backend side; but on reflection it's hard to see how it could cause any real confusion, so maybe we should just go with that. Another point is that the patch would be shorter and more reliable if you just forced whereToSendOutput = DestNone, without trying to save and restore it. Once the connection is known busted, there is no point in sending any future I/O towards the client, either. Makes sense, will change and commit. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Mon, May 7, 2012 at 12:39 PM, Magnus Hagander mag...@hagander.net wrote:
Makes sense, will change and commit.
Since the following hunk is repeated 3x, maybe it should be stuffed
into a function that is then called in three places:
+ if (IsTransactionState())
+ ereport(COMMERROR,
+ (errcode(ERRCODE_CONNECTION_FAILURE),
+errmsg(unexpected EOF on
client connection with an open transaction)));
+ else
+ {
+ /*
+* Can't send DEBUG log messages to client at
this point.
+* Since we're disconnecting right away, we
don't need to
+* restore whereToSendOutput.
+*/
+ whereToSendOutput = DestNone;
+ ereport(DEBUG1,
+
(errcode(ERRCODE_CONNECTION_DOES_NOT_EXIST),
+errmsg(unexpected EOF on
client connection)));
+ }
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Mon, May 7, 2012 at 7:18 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, May 7, 2012 at 12:39 PM, Magnus Hagander mag...@hagander.net wrote: Makes sense, will change and commit. Since the following hunk is repeated 3x, maybe it should be stuffed into a function that is then called in three places: I considered it trivial enough not to do that for it. I can perhaps be convinced otherwise, but I doubt it's worth it.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: On Mon, May 7, 2012 at 7:18 PM, Robert Haas robertmh...@gmail.com wrote: Since the following hunk is repeated 3x, maybe it should be stuffed into a function that is then called in three places: I considered it trivial enough not to do that for it. I can perhaps be convinced otherwise, but I doubt it's worth it.. I had considered suggesting the same, but decided not to on the grounds that if we fold these into a subroutine, it will no longer be possible to tell from the file-and-line-number info which call site reported the error. I'm not sure that there would be cases where we'd want to tell that, but I'm not sure there wouldn't be, either. So on the whole I agree with the way Magnus coded it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 1:26 PM, Magnus Hagander mag...@hagander.net wrote: I had a request from a customer asking if we could make a switch to specifically disable the unexpected EOF message that fills lots of peoples logs. Along the same way that we have a flag to turn off the nonstandard use of string escapes message that is another culprit (that's actually a much *worse* problem than just the unexpected EOF). The unexpected EOF message *does* indicate the client is doing something stupid, but it's not like it's an *actual problem* in pretty much every deployment out there... Would we consider adding such a switch (it should be easy enough to do), or do we want to push this off to the mythical let's improve the logging subsystem project that might eventually materialize if we're lucky? Meaning - would people object to such a switch? Yes, if the new parameter allows a generic filter on multiple user-specified message types. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 2:31 PM, Simon Riggs si...@2ndquadrant.com wrote: On Thu, May 3, 2012 at 1:26 PM, Magnus Hagander mag...@hagander.net wrote: I had a request from a customer asking if we could make a switch to specifically disable the unexpected EOF message that fills lots of peoples logs. Along the same way that we have a flag to turn off the nonstandard use of string escapes message that is another culprit (that's actually a much *worse* problem than just the unexpected EOF). The unexpected EOF message *does* indicate the client is doing something stupid, but it's not like it's an *actual problem* in pretty much every deployment out there... Would we consider adding such a switch (it should be easy enough to do), or do we want to push this off to the mythical let's improve the logging subsystem project that might eventually materialize if we're lucky? Meaning - would people object to such a switch? Yes, if the new parameter allows a generic filter on multiple user-specified message types. Uh, just to be clear, you object *if* it has the generic filter? Also, AFAIK we don't *have* a message type at this point (one of the things said mythical project wanted to look at), so the only thing we could really filter on would be the whole text of the message, no? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 2:31 PM, Simon Riggs si...@2ndquadrant.com wrote: Would we consider adding such a switch (it should be easy enough to do), or do we want to push this off to the mythical let's improve the logging subsystem project that might eventually materialize if we're lucky? Meaning - would people object to such a switch? Yes, if the new parameter allows a generic filter on multiple user-specified message types. Are you answering the Would we consider or the would people object?
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 2:34 PM, Vik Reykja vikrey...@gmail.com wrote: On Thu, May 3, 2012 at 2:31 PM, Simon Riggs si...@2ndquadrant.com wrote: Would we consider adding such a switch (it should be easy enough to do), or do we want to push this off to the mythical let's improve the logging subsystem project that might eventually materialize if we're lucky? Meaning - would people object to such a switch? Yes, if the new parameter allows a generic filter on multiple user-specified message types. Are you answering the Would we consider or the would people object? Oh, nice catch - I guess my phrasing of those two questions was really stupid :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander wrote: Also, AFAIK we don't *have* a message type at this point (one of the things said mythical project wanted to look at), so the only thing we could really filter on would be the whole text of the message, no? We have SQLSTATE, but this seems to be one of those situations where we've been sloppy about using the right value. We seem to be using '08P01' (protocol_violation), which is also used for finding the wrong bytes on a working connection. It seems to me a broken connection is exactly the case where you would expect to see '08006' (connection_failure). FWIW, there are also specific exceptions for rejecting a connection attempt, and for attempting to send something when no connection exists. We don't need to invent new mechanisms for categorizing messages; we just need to start consistently using the one we have correctly. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Simon Riggs si...@2ndquadrant.com writes: On Thu, May 3, 2012 at 1:26 PM, Magnus Hagander mag...@hagander.net wrote: I had a request from a customer asking if we could make a switch to specifically disable the unexpected EOF message that fills lots of peoples logs. Yes, if the new parameter allows a generic filter on multiple user-specified message types. I agree with Simon --- a disable for that specific message seems like a kluge, and an ugly one at that. (The right solution for this customer is to fix their broken application.) But a generic filter capability might be useful enough to justify its keep. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 2:46 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Magnus Hagander wrote: Also, AFAIK we don't *have* a message type at this point (one of the things said mythical project wanted to look at), so the only thing we could really filter on would be the whole text of the message, no? We have SQLSTATE, but this seems to be one of those situations where we've been sloppy about using the right value. We seem to be using '08P01' (protocol_violation), which is also used for finding the wrong bytes on a working connection. It seems to me a broken connection is exactly the case where you would expect to see '08006' (connection_failure). FWIW, there are also specific exceptions for rejecting a connection attempt, and for attempting to send something when no connection exists. We don't need to invent new mechanisms for categorizing messages; we just need to start consistently using the one we have correctly. While it might work a bit for this one, do we really expect to be able to map a single SQLSTATE to each single message at any point? Unless we can do that, it's never going to go all the way - though it might still be useful of course. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 4:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On Thu, May 3, 2012 at 1:26 PM, Magnus Hagander mag...@hagander.net wrote: I had a request from a customer asking if we could make a switch to specifically disable the unexpected EOF message that fills lots of peoples logs. Yes, if the new parameter allows a generic filter on multiple user-specified message types. I agree with Simon --- a disable for that specific message seems like a kluge, and an ugly one at that. (The right solution for this customer is to fix their broken application.) But a generic filter capability might be useful enough to justify its keep. Are you thinking basically regexp against the main text, or something else, when you say generic filter capacity? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 4:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: I agree with Simon --- a disable for that specific message seems like a kluge, and an ugly one at that. (The right solution for this customer is to fix their broken application.) But a generic filter capability might be useful enough to justify its keep. Are you thinking basically regexp against the main text, or something else, when you say generic filter capacity? In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 4:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: I agree with Simon --- a disable for that specific message seems like a kluge, and an ugly one at that. (The right solution for this customer is to fix their broken application.) But a generic filter capability might be useful enough to justify its keep. Are you thinking basically regexp against the main text, or something else, when you say generic filter capacity? In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. I'm worried it's not really granular enough. regexp-on-text would also have the advantage of being able to filter stuff coming from stored procedures or such as well - without having to invent a whole bunch of SQLSTATEs to put in the stored procedures (consider the usecase when somebody else wrote the stored procedures and the DBA wants to limit the logging). We could have two parameters of course - log_filter_sqlstate and log_filter_re or something like that... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: Are you thinking basically regexp against the main text, or something else, when you say generic filter capacity? In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. I'm worried it's not really granular enough. I dislike the idea of regex-on-text because of i18n issues. There's no guarantee for instance that all sessions are running with the same LC_MESSAGES locale. In any case, anybody who's dead set on doing it that way can do it today with grep. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Excerpts from Magnus Hagander's message of jue may 03 10:58:12 -0400 2012: On Thu, May 3, 2012 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote: In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. I'm worried it's not really granular enough. Yeah. regexp-on-text would also have the advantage of being able to filter stuff coming from stored procedures or such as well - without having to invent a whole bunch of SQLSTATEs to put in the stored procedures (consider the usecase when somebody else wrote the stored procedures and the DBA wants to limit the logging). We could have two parameters of course - log_filter_sqlstate and log_filter_re or something like that... The problem with regexes is that they are so expensive. You just need to forget the start anchor and it's suddenly a serious problem. And if you want to filter out a second message, the config option starts to become rather unwieldy. I wonder if there's a better way to selectively filter out messages -- say some sort of config file that contains a list of filenames/numbers of messages to disable. That particular idea would be a pain to maintain, of course, not to mention that it'd change from one release to the next. Hey, maybe we could add a UUID to each ereport() call site ;-) (Maybe the sites that have a load problem caused by log traffic are not the same sites that would like to filter out messages, and thus using regexes is not really a problem. It doesn't seem to be the kind of bet that we want to do.) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 11:20 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Hey, maybe we could add a UUID to each ereport() call site ;-) I can't help but feel we're designing a $10.00 solution to a $0.25 problem. I think I'd actually support adding something like a UUID to every ereport and a filtering mechanism that works on that basis. But let's face it: this particular message is exponentially more annoying than average. We're basically forcing application developers to jump through hoops to avoid filling the log with unnecessary chatter. I've spent a bunch of time trying to get rid of them in various past jobs, and I've never gotten any benefit out of having them. Maybe the solution is to just demote that particular message to DEBUG1 and declare that closing the connection is a perfectly sensible way for an application to indicate that the conversation is over. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Robert Haas robertmh...@gmail.com writes: On Thu, May 3, 2012 at 11:20 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Hey, maybe we could add a UUID to each ereport() call site ;-) I can't help but feel we're designing a $10.00 solution to a $0.25 problem. I think I'd actually support adding something like a UUID to every ereport and a filtering mechanism that works on that basis. But let's face it: this particular message is exponentially more annoying than average. We're basically forcing application developers to jump through hoops to avoid filling the log with unnecessary chatter. I've spent a bunch of time trying to get rid of them in various past jobs, and I've never gotten any benefit out of having them. Maybe the solution is to just demote that particular message to DEBUG1 and declare that closing the connection is a perfectly sensible way for an application to indicate that the conversation is over. I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 11:39 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, May 3, 2012 at 11:20 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Hey, maybe we could add a UUID to each ereport() call site ;-) I can't help but feel we're designing a $10.00 solution to a $0.25 problem. I think I'd actually support adding something like a UUID to every ereport and a filtering mechanism that works on that basis. But let's face it: this particular message is exponentially more annoying than average. We're basically forcing application developers to jump through hoops to avoid filling the log with unnecessary chatter. I've spent a bunch of time trying to get rid of them in various past jobs, and I've never gotten any benefit out of having them. Maybe the solution is to just demote that particular message to DEBUG1 and declare that closing the connection is a perfectly sensible way for an application to indicate that the conversation is over. I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. I agree. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Magnus Hagander's message: Tom Lane t...@sss.pgh.pa.us wrote: In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. I'm worried it's not really granular enough. Yeah. Just to be sure we're not inventing a problem here, can someone produce an example of a situation where it would not be granular enough (assuming we correct bad SQLSTATE choices where they exist)? I count 232 distinct SQLSTATE values (139 standard values and 93 PostgreSQL-specific values), and we can create more if we want them; although I would recommend against doing that to get finer resolution on a standard SQLSTATE value. A standard value which is too coarse would be the strongest argument for adding some other mechanism, IMO. If we do, I would be inclined toward something to identify distinct conditions within a SQLSTATE, rather than some overarching independent mechanism. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 11:46 AM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Magnus Hagander's message: Tom Lane t...@sss.pgh.pa.us wrote: In the context of yesterday's discussions, I wonder whether a filter by SQLSTATE would be appropriate. I'm worried it's not really granular enough. Yeah. Just to be sure we're not inventing a problem here, can someone produce an example of a situation where it would not be granular enough (assuming we correct bad SQLSTATE choices where they exist)? I count 232 distinct SQLSTATE values (139 standard values and 93 PostgreSQL-specific values), and we can create more if we want them; although I would recommend against doing that to get finer resolution on a standard SQLSTATE value. A standard value which is too coarse would be the strongest argument for adding some other mechanism, IMO. If we do, I would be inclined toward something to identify distinct conditions within a SQLSTATE, rather than some overarching independent mechanism. Well, nearby Tom and I discussed demoting the message to DEBUG1 when no transaction is in progress. Presumably the two messages would share the same SQL state, unless we're going to create separate SQL states for connection-closed-not-in-a-txn and connection-closed-in-a-txn; and yet I think there's a very decent argument that you're much more likely to care about the latter than the former. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Robert Haas robertmh...@gmail.com writes: Well, nearby Tom and I discussed demoting the message to DEBUG1 when no transaction is in progress. Presumably the two messages would share the same SQL state, unless we're going to create separate SQL states for connection-closed-not-in-a-txn and connection-closed-in-a-txn; and yet I think there's a very decent argument that you're much more likely to care about the latter than the former. If we're going to treat the two cases differently then assigning distinct SQLSTATEs seems entirely reasonable to me. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 5:39 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, May 3, 2012 at 11:20 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Hey, maybe we could add a UUID to each ereport() call site ;-) I can't help but feel we're designing a $10.00 solution to a $0.25 problem. I think I'd actually support adding something like a UUID to every ereport and a filtering mechanism that works on that basis. But let's face it: this particular message is exponentially more annoying than average. We're basically forcing application developers to jump through hoops to avoid filling the log with unnecessary chatter. I've spent a bunch of time trying to get rid of them in various past jobs, and I've never gotten any benefit out of having them. Maybe the solution is to just demote that particular message to DEBUG1 and declare that closing the connection is a perfectly sensible way for an application to indicate that the conversation is over. I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. I agree - that would certainly be a good fix for this one. One question is do we want something like this: - ereport(COMMERROR, + ereport(IsTransactionState() ? COMMERROR : DEBUG1, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg(unexpected EOF on client connection))); (in a couple of places, yes) or do we want to make the text of the error message different as well, saying something like unexpected EOF on client connection with an open transaction? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 5:39 PM, Tom Lane t...@sss.pgh.pa.us wrote: I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. I agree - that would certainly be a good fix for this one. One question is do we want something like this: - ereport(COMMERROR, + ereport(IsTransactionState() ? COMMERROR : DEBUG1, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg(unexpected EOF on client connection))); or do we want to make the text of the error message different as well, saying something like unexpected EOF on client connection with an open transaction? I'd vote for different texts and different SQLSTATEs too, per other discussion. (I think we'd decided that ERRCODE_PROTOCOL_VIOLATION was a bad choice anyway.) Also, I'm afraid that the above patch probably doesn't work as-is; won't elog.c try to send the DEBUG1 message to the client? I think you'll need some additional code to shut down error message output first. Resetting whereToSendOutput is probably sufficient. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Well, nearby Tom and I discussed demoting the message to DEBUG1 when no transaction is in progress. Presumably the two messages would share the same SQL state, unless we're going to create separate SQL states for connection-closed-not-in-a-txn and connection-closed-in-a-txn; and yet I think there's a very decent argument that you're much more likely to care about the latter than the former. If we're going to treat the two cases differently then assigning distinct SQLSTATEs seems entirely reasonable to me. Would it make sense to use 08003 (connection_does_not_exist) when a broken connection for an idle process is discovered, and 08006 (connection_failure) for the in transaction failure? What about a failure just after COMMIT and before successfully sending that result to the client? I notice there's a SQLSTATE 08007 (transaction_resolution_unknown), but I don't know whether that makes sense on the server side, or just on the client side. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Kevin Grittner kevin.gritt...@wicourts.gov writes: Would it make sense to use 08003 (connection_does_not_exist) when a broken connection for an idle process is discovered, and 08006 (connection_failure) for the in transaction failure? What about a failure just after COMMIT and before successfully sending that result to the client? I notice there's a SQLSTATE 08007 (transaction_resolution_unknown), but I don't know whether that makes sense on the server side, or just on the client side. AFAICS, all the 08 class is meant to be issued by client-side code, not the server. I think we probably have to use nonstandard SQLSTATEs for these messages. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 7:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 5:39 PM, Tom Lane t...@sss.pgh.pa.us wrote: I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. I agree - that would certainly be a good fix for this one. One question is do we want something like this: - ereport(COMMERROR, + ereport(IsTransactionState() ? COMMERROR : DEBUG1, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg(unexpected EOF on client connection))); or do we want to make the text of the error message different as well, saying something like unexpected EOF on client connection with an open transaction? I'd vote for different texts and different SQLSTATEs too, per other discussion. (I think we'd decided that ERRCODE_PROTOCOL_VIOLATION was a bad choice anyway.) Also, I'm afraid that the above patch probably doesn't work as-is; won't elog.c try to send the DEBUG1 message to the client? I think you'll need some additional code to shut down error message output first. Resetting whereToSendOutput is probably sufficient. Yeah, I didn't go as far as testing it - there's also more than one spot where we log it... I'll cook up a patch. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 7:21 PM, Magnus Hagander mag...@hagander.net wrote: On Thu, May 3, 2012 at 7:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: On Thu, May 3, 2012 at 5:39 PM, Tom Lane t...@sss.pgh.pa.us wrote: I could support that with one tweak: it's only DEBUG1 if you don't have an open transaction. Dropping the connection while in a transaction *is* an application bug; I don't care how lazy the app programmer is feeling. I agree - that would certainly be a good fix for this one. One question is do we want something like this: - ereport(COMMERROR, + ereport(IsTransactionState() ? COMMERROR : DEBUG1, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg(unexpected EOF on client connection))); or do we want to make the text of the error message different as well, saying something like unexpected EOF on client connection with an open transaction? I'd vote for different texts and different SQLSTATEs too, per other discussion. (I think we'd decided that ERRCODE_PROTOCOL_VIOLATION was a bad choice anyway.) Also, I'm afraid that the above patch probably doesn't work as-is; won't elog.c try to send the DEBUG1 message to the client? I think you'll need some additional code to shut down error message output first. Resetting whereToSendOutput is probably sufficient. Yeah, I didn't go as far as testing it - there's also more than one spot where we log it... I'll cook up a patch. Heh - we already used ERRCODE_CONNECTION_FAILURE on the errors in copy.c. Since COPY can only happen when there is a transaction (right?), I just changed those error messages for consistency. This patch works through my testing - can anyone spot a hole in it still? The next question is - of course - whether we can sneak this in before beta... -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ ereport_eof.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Tom Lane t...@sss.pgh.pa.us wrote: AFAICS, all the 08 class is meant to be issued by client-side code, not the server. I think we probably have to use nonstandard SQLSTATEs for these messages. OK, if we're going that route, how about using Class 2D * Invalid Transaction Termination? I still think it might be useful to differentiate in our server log between the case where the transaction failed and the case where the transaction committed but we don't know that the client got the news of that. How about something like: 2DP01 connection_lost_during_transaction 2DP02 connection_lost_during_commit_notification I'm less sure what makes sense if the connection fails while idle (not in transaction). If you don't like Class 08 * Connection Exception for that, I'm not quite sure where it belongs. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Magnus Hagander mag...@hagander.net writes: Heh - we already used ERRCODE_CONNECTION_FAILURE on the errors in copy.c. Since COPY can only happen when there is a transaction (right?), I just changed those error messages for consistency. Agreed on changing the message texts to match, but I wonder whether we ought not switch all those SQLSTATEs to something different. Per my comment to Kevin, I think the whole 08 class is meant to be issued on the client side. Maybe it's okay to conflate a server-detected connection loss with client-detected loss, but I'm not convinced. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
Kevin Grittner kevin.gritt...@wicourts.gov writes: I still think it might be useful to differentiate in our server log between the case where the transaction failed and the case where the transaction committed but we don't know that the client got the news of that. How about something like: 2DP01 connection_lost_during_transaction 2DP02 connection_lost_during_commit_notification That would be a useful distinction, but I'm not sure how easily our code can make it. I'm less sure what makes sense if the connection fails while idle (not in transaction). If you don't like Class 08 * Connection Exception for that, I'm not quite sure where it belongs. I'm not convinced that these cases belong in any of the standard's classes. IMO the standard is only standardizing application-visible error cases, which these are not. In particular I think class 2D is not appropriate, since AFAICS the standard means that to pertain to incorrect issuance of a COMMIT or ROLLBACK command. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unexpected EOF messages
On Thu, May 3, 2012 at 7:48 PM, Tom Lane t...@sss.pgh.pa.us wrote: Magnus Hagander mag...@hagander.net writes: Heh - we already used ERRCODE_CONNECTION_FAILURE on the errors in copy.c. Since COPY can only happen when there is a transaction (right?), I just changed those error messages for consistency. Agreed on changing the message texts to match, but I wonder whether we ought not switch all those SQLSTATEs to something different. Per my comment to Kevin, I think the whole 08 class is meant to be issued on the client side. Maybe it's okay to conflate a server-detected connection loss with client-detected loss, but I'm not convinced. Sure,that's a simple search and replace of course... If we can come to a decision about what codes to actually use. I'm not sure I have much input other than that I agree they need to be different :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
