public inbox for [email protected]help / color / mirror / Atom feed
FATAL: simple query "BEGIN" arrived before ending an extended query message 8+ messages / 2 participants [nested] [flat]
* FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-09 11:13 Emond Papegaaij <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Emond Papegaaij @ 2025-07-09 11:13 UTC (permalink / raw) To: [email protected] Hi all, Recently we started seeing errors from pgpool-II stating 'FATAL: simple query "BEGIN" arrived before ending an extended query message'. These errors started occurring after upgrading PgJDBC from 42.7.5 to 42.7.7. I've created the following ticket at PgJDBC: https://github.com/pgjdbc/pgjdbc/issues/3724 . It was closed as a duplicate of https://github.com/pgjdbc/pgjdbc/issues/3107 , which describes a similar issue and was opened by Tatsuo Ishii. The problem however, is that previously the error was only triggered when using autosave, which is not a common setup, but now the error is triggered even when using default configuration. This makes it impossible to combine PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on this from the side of PostgreSQL and PgJDBC, but I just wanted to raise some awareness on this issue here. IMHO PostgreSQL should either explicitly allow this (in which case pgpool-II needs to be fixed) or explicitly disallow it (in which case PgJDBC needs to be fixed). The current situation is no good as we now simply cannot upgrade PgJDBC anymore (and the same will be true for all other users of pgpool-II). Best regards, Emond Papegaaij ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-10 02:55 Tatsuo Ishii <[email protected]> parent: Emond Papegaaij <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tatsuo Ishii @ 2025-07-10 02:55 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] > Hi all, > > Recently we started seeing errors from pgpool-II stating 'FATAL: simple > query "BEGIN" arrived before ending an extended query message'. These > errors started occurring after upgrading PgJDBC from 42.7.5 to 42.7.7. I've > created the following ticket at PgJDBC: > https://github.com/pgjdbc/pgjdbc/issues/3724 . It was closed as a duplicate > of https://github.com/pgjdbc/pgjdbc/issues/3107 , which describes a similar > issue and was opened by Tatsuo Ishii. > > The problem however, is that previously the error was only triggered when > using autosave, which is not a common setup, but now the error is triggered > even when using default configuration. This makes it impossible to combine > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on > this from the side of PostgreSQL and PgJDBC, but I just wanted to raise > some awareness on this issue here. IMHO PostgreSQL should either explicitly > allow this (in which case pgpool-II needs to be fixed) or explicitly > disallow it (in which case PgJDBC needs to be fixed). As far as I know about PostgreSQL's side, Tom Lane said about this: https://www.postgresql.org/message-id/[email protected] > I think it's poor practice, at best. You should end the > extended-protocol query cycle before invoking simple query. > > I'm disinclined to document, or make any promises about, > what happens if you mix the protocols. In my understanding he does not say PostgreSQL explicitely allows this (mixing extended and simple protocol message). > The current situation > is no good as we now simply cannot upgrade PgJDBC anymore (and the same > will be true for all other users of pgpool-II). Yeah. What I don't understand is, why PgJDBC decided to make it default (sending simple protocol query after extended query protocl without sync) even without autosave being set when they update PgJDBC to 42.7.7. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-10 06:34 Emond Papegaaij <[email protected]> parent: Tatsuo Ishii <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Emond Papegaaij @ 2025-07-10 06:34 UTC (permalink / raw) To: Tatsuo Ishii <[email protected]>; +Cc: [email protected] > > > The problem however, is that previously the error was only triggered when > > using autosave, which is not a common setup, but now the error is > triggered > > even when using default configuration. This makes it impossible to > combine > > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on > > this from the side of PostgreSQL and PgJDBC, but I just wanted to raise > > some awareness on this issue here. IMHO PostgreSQL should either > explicitly > > allow this (in which case pgpool-II needs to be fixed) or explicitly > > disallow it (in which case PgJDBC needs to be fixed). > > As far as I know about PostgreSQL's side, Tom Lane said about this: > https://www.postgresql.org/message-id/[email protected] > > > I think it's poor practice, at best. You should end the > > extended-protocol query cycle before invoking simple query. > > > > I'm disinclined to document, or make any promises about, > > what happens if you mix the protocols. > > In my understanding he does not say PostgreSQL explicitely allows this > (mixing extended and simple protocol message). > It's more or less allowed without any errors, but the expected behavior is not clear nor is it documented. It seems libpq even throws an error on the client side when you try to do this: Jelte Fennema-Nio in https://www.postgresql.org/message-id/CAGECzQQ1hs2DU9pmQq18Y%3DqK4nZqhXDVg-sGEa5K01Lj4XMmxw%40mail.g... > I totally agree that it makes sense to throw an error in this case. > Libpq actually throws an error client side when a caller attempts to > do this, but this is something that should be checked server side, > given that the protocol docs specify this: > docs> At completion of each series of extended-query messages, the > frontend should issue a Sync message. > > The current situation > > is no good as we now simply cannot upgrade PgJDBC anymore (and the same > > will be true for all other users of pgpool-II). > > Yeah. > > What I don't understand is, why PgJDBC decided to make it default > (sending simple protocol query after extended query protocl without > sync) even without autosave being set when they update PgJDBC to > 42.7.7. > It wasn't a real decision to make this happen more often. They fixed a bug that caused many queries to be sent using the extended protocol even when explicitly asked for a simple query: https://github.com/pgjdbc/pgjdbc/issues/3724#issuecomment-3051773696 In our case we got sporadic errors. These errors are very hard to reproduce. Some users were not able to login into our application at all, others had no trouble whatsoever and some users got an error every once in a while. Even when tests seem fine, you might get these errors later on. None of our automated tests failed. On our testing environment we've only seen the error twice in a period of 3 weeks. When we moved to production, we triggered the error over 100 times in just 1 day! I would certainly not recommend using PgJDBC 42.7.6 and up when using pgpool. Best regards, Emond ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-21 04:12 Tatsuo Ishii <[email protected]> parent: Emond Papegaaij <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tatsuo Ishii @ 2025-07-21 04:12 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] Hi Emond, >> > The problem however, is that previously the error was only triggered when >> > using autosave, which is not a common setup, but now the error is >> triggered >> > even when using default configuration. This makes it impossible to >> combine >> > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on >> > this from the side of PostgreSQL and PgJDBC, but I just wanted to raise >> > some awareness on this issue here. IMHO PostgreSQL should either >> explicitly >> > allow this (in which case pgpool-II needs to be fixed) or explicitly >> > disallow it (in which case PgJDBC needs to be fixed). >> >> As far as I know about PostgreSQL's side, Tom Lane said about this: >> https://www.postgresql.org/message-id/[email protected] >> >> > I think it's poor practice, at best. You should end the >> > extended-protocol query cycle before invoking simple query. >> > >> > I'm disinclined to document, or make any promises about, >> > what happens if you mix the protocols. >> >> In my understanding he does not say PostgreSQL explicitely allows this >> (mixing extended and simple protocol message). >> > > It's more or less allowed without any errors, but the expected behavior is > not clear nor is it documented. It seems libpq even throws an error on the > client side when you try to do this: > > Jelte Fennema-Nio in > https://www.postgresql.org/message-id/CAGECzQQ1hs2DU9pmQq18Y%3DqK4nZqhXDVg-sGEa5K01Lj4XMmxw%40mail.g... >> I totally agree that it makes sense to throw an error in this case. >> Libpq actually throws an error client side when a caller attempts to >> do this, but this is something that should be checked server side, >> given that the protocol docs specify this: >> docs> At completion of each series of extended-query messages, the >> frontend should issue a Sync message. > > >> > The current situation >> > is no good as we now simply cannot upgrade PgJDBC anymore (and the same >> > will be true for all other users of pgpool-II). >> >> Yeah. >> >> What I don't understand is, why PgJDBC decided to make it default >> (sending simple protocol query after extended query protocl without >> sync) even without autosave being set when they update PgJDBC to >> 42.7.7. >> > > It wasn't a real decision to make this happen more often. They fixed a bug > that caused many queries to be sent using the extended protocol even when > explicitly asked for a simple query: > https://github.com/pgjdbc/pgjdbc/issues/3724#issuecomment-3051773696 > > In our case we got sporadic errors. These errors are very hard to > reproduce. Some users were not able to login into our application at all, > others had no trouble whatsoever and some users got an error every once in > a while. Even when tests seem fine, you might get these errors later on. > None of our automated tests failed. On our testing environment we've only > seen the error twice in a period of 3 weeks. When we moved to production, > we triggered the error over 100 times in just 1 day! I would certainly not > recommend using PgJDBC 42.7.6 and up when using pgpool. I created a patch for pgpool to allow a simple query without ending a sequence of extended query protocol message by a sync message. The idea is, within SimpleQuery() (a function responsible for processing simple protocol query), check whether extended query protocol ends. If not, call ProcessBackendReposnse() to process any replies from backend such as parse complete, bind complete or command complete and so. After this, SimpleQuery() can process simple query as usual. I hope this solves the issue with PgJDBC 42.7.6. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp Attachments: [application/octet-stream] allow_simple_query_after_extended_query.patch (2.2K, 2-allow_simple_query_after_extended_query.patch) download | inline diff: diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index 11befe979..e030e7b6d 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -222,20 +222,39 @@ SimpleQuery(POOL_CONNECTION *frontend, strlcpy(query_string_buffer, contents, sizeof(query_string_buffer)); /* - * Check if extended query protocol message ended. If not, reject the - * query and raise an error to terminate the session to avoid hanging up. - * However if we are processing a reset query (frontend == NULL), we skip - * the check as we don't want to raise a error. + * Check if extended query protocol message ended. If not, process any + * pending response from backend using ProcessBackendResponse(). However + * if we are processing a reset query (frontend == NULL), we skip the + * check as we don't need to care about any pending response from backend. */ if (SL_MODE) { + int state; + short num_fields; + if (frontend != NULL && (pool_is_doing_extended_query_message() || - pool_pending_message_head_message())) - - ereport(FATAL, + pool_pending_message_exists())) + { + ereport(DEBUG1, (errmsg("simple query \"%s\" arrived before ending an extended query message", query_string_buffer))); + + /* if pending message exists, process it */ + while (pool_pending_message_exists()) + { + /* + * read_kind_from_backend requires that query is inprogress + * and doing extended query state because it needs to refer to + * proper query context in session context. + */ + pool_set_query_in_progress(); + pool_set_doing_extended_query_message(); + /* process pending responses from backend */ + ProcessBackendResponse(frontend, backend, &state, &num_fields); + } + } + pool_unset_doing_extended_query_message(); } /* show ps status */ @@ -3080,6 +3099,12 @@ ProcessFrontendResponse(POOL_CONNECTION *frontend, return status; } +/* + * Read one backend response and process it. + * + * state: used for processing reset query + * num_fields: used in V2 protocol + */ POOL_STATUS ProcessBackendResponse(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend, ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-23 08:03 Tatsuo Ishii <[email protected]> parent: Tatsuo Ishii <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tatsuo Ishii @ 2025-07-23 08:03 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] > Hi Emond, > >>> > The problem however, is that previously the error was only triggered when >>> > using autosave, which is not a common setup, but now the error is >>> triggered >>> > even when using default configuration. This makes it impossible to >>> combine >>> > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on >>> > this from the side of PostgreSQL and PgJDBC, but I just wanted to raise >>> > some awareness on this issue here. IMHO PostgreSQL should either >>> explicitly >>> > allow this (in which case pgpool-II needs to be fixed) or explicitly >>> > disallow it (in which case PgJDBC needs to be fixed). >>> >>> As far as I know about PostgreSQL's side, Tom Lane said about this: >>> https://www.postgresql.org/message-id/[email protected] >>> >>> > I think it's poor practice, at best. You should end the >>> > extended-protocol query cycle before invoking simple query. >>> > >>> > I'm disinclined to document, or make any promises about, >>> > what happens if you mix the protocols. >>> >>> In my understanding he does not say PostgreSQL explicitely allows this >>> (mixing extended and simple protocol message). >>> >> >> It's more or less allowed without any errors, but the expected behavior is >> not clear nor is it documented. It seems libpq even throws an error on the >> client side when you try to do this: >> >> Jelte Fennema-Nio in >> https://www.postgresql.org/message-id/CAGECzQQ1hs2DU9pmQq18Y%3DqK4nZqhXDVg-sGEa5K01Lj4XMmxw%40mail.g... >>> I totally agree that it makes sense to throw an error in this case. >>> Libpq actually throws an error client side when a caller attempts to >>> do this, but this is something that should be checked server side, >>> given that the protocol docs specify this: >>> docs> At completion of each series of extended-query messages, the >>> frontend should issue a Sync message. >> >> >>> > The current situation >>> > is no good as we now simply cannot upgrade PgJDBC anymore (and the same >>> > will be true for all other users of pgpool-II). >>> >>> Yeah. >>> >>> What I don't understand is, why PgJDBC decided to make it default >>> (sending simple protocol query after extended query protocl without >>> sync) even without autosave being set when they update PgJDBC to >>> 42.7.7. >>> >> >> It wasn't a real decision to make this happen more often. They fixed a bug >> that caused many queries to be sent using the extended protocol even when >> explicitly asked for a simple query: >> https://github.com/pgjdbc/pgjdbc/issues/3724#issuecomment-3051773696 >> >> In our case we got sporadic errors. These errors are very hard to >> reproduce. Some users were not able to login into our application at all, >> others had no trouble whatsoever and some users got an error every once in >> a while. Even when tests seem fine, you might get these errors later on. >> None of our automated tests failed. On our testing environment we've only >> seen the error twice in a period of 3 weeks. When we moved to production, >> we triggered the error over 100 times in just 1 day! I would certainly not >> recommend using PgJDBC 42.7.6 and up when using pgpool. > > I created a patch for pgpool to allow a simple query without ending a > sequence of extended query protocol message by a sync message. > > The idea is, within SimpleQuery() (a function responsible for > processing simple protocol query), check whether extended query > protocol ends. If not, call ProcessBackendReposnse() to process any > replies from backend such as parse complete, bind complete or command > complete and so. After this, SimpleQuery() can process simple query > as usual. I hope this solves the issue with PgJDBC 42.7.6. After thinking more, I concluded that it would be better to do the task in ProcessFrontendReposnse(), rather than in SimpleQuery() because we usually do not call ProcessBackendResponse() inside functions such as SimpleQuery() that are responsible for processing particular frontend message. Attached is the v2 patch. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp Attachments: [application/octet-stream] v2-0001-Allow-to-accept-simple-query-even-if-extended-que.patch (4.3K, 2-v2-0001-Allow-to-accept-simple-query-even-if-extended-que.patch) download | inline diff: From a2ec0d2326a562576905f14d75391d22a0a070b3 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii <[email protected]> Date: Wed, 23 Jul 2025 16:53:03 +0900 Subject: [PATCH v2] Allow to accept simple query even if extended queries do not end. Recently pgJDBC has started to issue simple query without ending extended query protocol sequences with sync. This brought a disaster to pgpool. So pgpool refuses to accept it. https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=240c668d120065534b1d298d6facc86839fcbab9 However the situation got worse. Previously pgJDBC issued a simple query without ending extended protocol only when "autosave=always" option is given. But it was reported that pgJDBC has started to use it extensively. https://www.postgresql.org/message-id/CAGXsc%2Baoabb2xxyfckrHfTx4da8%3Ds1L9ai%2BY%2BuAS4cBPRMQD2A%40mail.gmail.com So this commit deals with the situation. When a simple query arrives without finishing extended protocol sequences, SimpleQuery() calls ProcessBackendResponse() to process pending extended query protocol replies from backend such as parse, bind or command complete messages, instead of rejecting the simple query. After this, SimpleQuery() process a simple query as usual. Discussion: https://www.postgresql.org/message-id/CAGXsc%2Baoabb2xxyfckrHfTx4da8%3Ds1L9ai%2BY%2BuAS4cBPRMQD2A%40mail.gmail.com --- src/protocol/pool_proto_modules.c | 61 ++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index 11befe979..fdb41bbf0 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -221,23 +221,6 @@ SimpleQuery(POOL_CONNECTION *frontend, /* save last query string for logging purpose */ strlcpy(query_string_buffer, contents, sizeof(query_string_buffer)); - /* - * Check if extended query protocol message ended. If not, reject the - * query and raise an error to terminate the session to avoid hanging up. - * However if we are processing a reset query (frontend == NULL), we skip - * the check as we don't want to raise a error. - */ - if (SL_MODE) - { - if (frontend != NULL && - (pool_is_doing_extended_query_message() || - pool_pending_message_head_message())) - - ereport(FATAL, - (errmsg("simple query \"%s\" arrived before ending an extended query message", - query_string_buffer))); - } - /* show ps status */ query_ps_status(contents, backend); @@ -2930,6 +2913,44 @@ ProcessFrontendResponse(POOL_CONNECTION *frontend, ereport(LOG, (errmsg("Query message from frontend."), errdetail("query: \"%s\"", contents))); + + /* + * Check if extended query protocol message ended. If not, process + * any pending response from backend using + * ProcessBackendResponse(). However if we are processing a reset + * query (frontend == NULL), we skip the check as we don't need to + * care about any pending response from backend. + */ + if (SL_MODE) + { + int state; + short num_fields; + + if (frontend != NULL && + (pool_is_doing_extended_query_message() || + pool_pending_message_exists())) + { + ereport(DEBUG1, + (errmsg("simple query \"%s\" arrived before ending an extended query message", + contents))); + + /* if pending message exists, process it */ + while (pool_pending_message_exists()) + { + /* + * read_kind_from_backend requires that query is + * inprogress and doing extended query state because + * it needs to refer to proper query context in + * session context. + */ + pool_set_query_in_progress(); + pool_set_doing_extended_query_message(); + /* process pending responses from backend */ + ProcessBackendResponse(frontend, backend, &state, &num_fields); + } + } + pool_unset_doing_extended_query_message(); + } status = SimpleQuery(frontend, backend, len, contents); break; @@ -3080,6 +3101,12 @@ ProcessFrontendResponse(POOL_CONNECTION *frontend, return status; } +/* + * Read one backend response and process it. + * + * state: used for processing reset query + * num_fields: used in V2 protocol + */ POOL_STATUS ProcessBackendResponse(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend, -- 2.25.1 ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-24 01:36 Tatsuo Ishii <[email protected]> parent: Tatsuo Ishii <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Tatsuo Ishii @ 2025-07-24 01:36 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] >> Hi Emond, >> >>>> > The problem however, is that previously the error was only triggered when >>>> > using autosave, which is not a common setup, but now the error is >>>> triggered >>>> > even when using default configuration. This makes it impossible to >>>> combine >>>> > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans are on >>>> > this from the side of PostgreSQL and PgJDBC, but I just wanted to raise >>>> > some awareness on this issue here. IMHO PostgreSQL should either >>>> explicitly >>>> > allow this (in which case pgpool-II needs to be fixed) or explicitly >>>> > disallow it (in which case PgJDBC needs to be fixed). >>>> >>>> As far as I know about PostgreSQL's side, Tom Lane said about this: >>>> https://www.postgresql.org/message-id/[email protected] >>>> >>>> > I think it's poor practice, at best. You should end the >>>> > extended-protocol query cycle before invoking simple query. >>>> > >>>> > I'm disinclined to document, or make any promises about, >>>> > what happens if you mix the protocols. >>>> >>>> In my understanding he does not say PostgreSQL explicitely allows this >>>> (mixing extended and simple protocol message). >>>> >>> >>> It's more or less allowed without any errors, but the expected behavior is >>> not clear nor is it documented. It seems libpq even throws an error on the >>> client side when you try to do this: >>> >>> Jelte Fennema-Nio in >>> https://www.postgresql.org/message-id/CAGECzQQ1hs2DU9pmQq18Y%3DqK4nZqhXDVg-sGEa5K01Lj4XMmxw%40mail.g... >>>> I totally agree that it makes sense to throw an error in this case. >>>> Libpq actually throws an error client side when a caller attempts to >>>> do this, but this is something that should be checked server side, >>>> given that the protocol docs specify this: >>>> docs> At completion of each series of extended-query messages, the >>>> frontend should issue a Sync message. >>> >>> >>>> > The current situation >>>> > is no good as we now simply cannot upgrade PgJDBC anymore (and the same >>>> > will be true for all other users of pgpool-II). >>>> >>>> Yeah. >>>> >>>> What I don't understand is, why PgJDBC decided to make it default >>>> (sending simple protocol query after extended query protocl without >>>> sync) even without autosave being set when they update PgJDBC to >>>> 42.7.7. >>>> >>> >>> It wasn't a real decision to make this happen more often. They fixed a bug >>> that caused many queries to be sent using the extended protocol even when >>> explicitly asked for a simple query: >>> https://github.com/pgjdbc/pgjdbc/issues/3724#issuecomment-3051773696 >>> >>> In our case we got sporadic errors. These errors are very hard to >>> reproduce. Some users were not able to login into our application at all, >>> others had no trouble whatsoever and some users got an error every once in >>> a while. Even when tests seem fine, you might get these errors later on. >>> None of our automated tests failed. On our testing environment we've only >>> seen the error twice in a period of 3 weeks. When we moved to production, >>> we triggered the error over 100 times in just 1 day! I would certainly not >>> recommend using PgJDBC 42.7.6 and up when using pgpool. >> >> I created a patch for pgpool to allow a simple query without ending a >> sequence of extended query protocol message by a sync message. >> >> The idea is, within SimpleQuery() (a function responsible for >> processing simple protocol query), check whether extended query >> protocol ends. If not, call ProcessBackendReposnse() to process any >> replies from backend such as parse complete, bind complete or command >> complete and so. After this, SimpleQuery() can process simple query >> as usual. I hope this solves the issue with PgJDBC 42.7.6. > > After thinking more, I concluded that it would be better to do the > task in ProcessFrontendReposnse(), rather than in SimpleQuery() > because we usually do not call ProcessBackendResponse() inside > functions such as SimpleQuery() that are responsible for processing > particular frontend message. > > Attached is the v2 patch. This is the v3 patch. Fix regression test 082.guard_against_bad_protocol. Slightly modify commit message. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp Attachments: [application/octet-stream] v3-0001-Allow-to-accept-simple-query-even-if-extended-que.patch (5.6K, 2-v3-0001-Allow-to-accept-simple-query-even-if-extended-que.patch) download | inline diff: From 8444e287c12676adee6bd27328bb9cea261f46d1 Mon Sep 17 00:00:00 2001 From: Tatsuo Ishii <[email protected]> Date: Thu, 24 Jul 2025 10:34:59 +0900 Subject: [PATCH v3] Allow to accept simple query even if extended queries do not end. Recently pgJDBC has started to issue simple query without ending extended query protocol sequences with sync. This brought a disaster to pgpool. So pgpool refused to accept it. https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=240c668d120065534b1d298d6facc86839fcbab9 However the situation got worse. Previously pgJDBC issued a simple query without ending extended protocol only when "autosave=always" option is given. But it was reported that pgJDBC has started to use it extensively. https://www.postgresql.org/message-id/CAGXsc%2Baoabb2xxyfckrHfTx4da8%3Ds1L9ai%2BY%2BuAS4cBPRMQD2A%40mail.gmail.com So this commit deals with the situation. When a simple query arrives without finishing extended protocol sequences, before calling SimpleQuery(), ProcessBackendResponse() is called to process pending extended query protocol replies from backend such as parse, bind or command complete messages, instead of rejecting the simple query. After this, SimpleQuery() can process the simple query as usual. Author: Tatsuo Ishii <[email protected]> Reported-by: Emond Papegaaij <[email protected]> Discussion: https://www.postgresql.org/message-id/CAGXsc%2Baoabb2xxyfckrHfTx4da8%3Ds1L9ai%2BY%2BuAS4cBPRMQD2A%40mail.gmail.com --- src/protocol/pool_proto_modules.c | 61 +++++++++++++------ .../082.guard_against_bad_protocol/test.sh | 5 +- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c index 11befe979..fdb41bbf0 100644 --- a/src/protocol/pool_proto_modules.c +++ b/src/protocol/pool_proto_modules.c @@ -221,23 +221,6 @@ SimpleQuery(POOL_CONNECTION *frontend, /* save last query string for logging purpose */ strlcpy(query_string_buffer, contents, sizeof(query_string_buffer)); - /* - * Check if extended query protocol message ended. If not, reject the - * query and raise an error to terminate the session to avoid hanging up. - * However if we are processing a reset query (frontend == NULL), we skip - * the check as we don't want to raise a error. - */ - if (SL_MODE) - { - if (frontend != NULL && - (pool_is_doing_extended_query_message() || - pool_pending_message_head_message())) - - ereport(FATAL, - (errmsg("simple query \"%s\" arrived before ending an extended query message", - query_string_buffer))); - } - /* show ps status */ query_ps_status(contents, backend); @@ -2930,6 +2913,44 @@ ProcessFrontendResponse(POOL_CONNECTION *frontend, ereport(LOG, (errmsg("Query message from frontend."), errdetail("query: \"%s\"", contents))); + + /* + * Check if extended query protocol message ended. If not, process + * any pending response from backend using + * ProcessBackendResponse(). However if we are processing a reset + * query (frontend == NULL), we skip the check as we don't need to + * care about any pending response from backend. + */ + if (SL_MODE) + { + int state; + short num_fields; + + if (frontend != NULL && + (pool_is_doing_extended_query_message() || + pool_pending_message_exists())) + { + ereport(DEBUG1, + (errmsg("simple query \"%s\" arrived before ending an extended query message", + contents))); + + /* if pending message exists, process it */ + while (pool_pending_message_exists()) + { + /* + * read_kind_from_backend requires that query is + * inprogress and doing extended query state because + * it needs to refer to proper query context in + * session context. + */ + pool_set_query_in_progress(); + pool_set_doing_extended_query_message(); + /* process pending responses from backend */ + ProcessBackendResponse(frontend, backend, &state, &num_fields); + } + } + pool_unset_doing_extended_query_message(); + } status = SimpleQuery(frontend, backend, len, contents); break; @@ -3080,6 +3101,12 @@ ProcessFrontendResponse(POOL_CONNECTION *frontend, return status; } +/* + * Read one backend response and process it. + * + * state: used for processing reset query + * num_fields: used in V2 protocol + */ POOL_STATUS ProcessBackendResponse(POOL_CONNECTION *frontend, POOL_CONNECTION_POOL *backend, diff --git a/src/test/regression/tests/082.guard_against_bad_protocol/test.sh b/src/test/regression/tests/082.guard_against_bad_protocol/test.sh index f1d16e6f3..c4f6e90b4 100755 --- a/src/test/regression/tests/082.guard_against_bad_protocol/test.sh +++ b/src/test/regression/tests/082.guard_against_bad_protocol/test.sh @@ -22,6 +22,7 @@ echo -n "creating test environment..." $PGPOOL_SETUP || exit 1 echo "done." echo "backend_weight1=0" >> etc/pgpool.conf +echo "client_min_messages=debug1" >> etc/pgpool.conf source ./bashrc.ports ./startall wait_for_pgpool_startup @@ -29,8 +30,8 @@ wait_for_pgpool_startup # test1: # Wait for 1 seconds before pgproto ended. # Usually 1 seconds should be enough to finish pgproto. -# If test suceeded, pgpool emits an error message: -# "FATAL: simple query "SAVEPOINT PGJDBC_AUTOSAVE" arrived before ending an extended query message" +# If the test suceeds, pgpool emits a debug message: +# "simple query "SAVEPOINT PGJDBC_AUTOSAVE" arrived before ending an extended query message" # grep command below should catch the message. timeout 1 $PGPROTO -d $PGDATABASE -p $PGPOOL_PORT -f ../pgproto.data |& grep 'simple query "SAVEPOINT PGJDBC_AUTOSAVE" arrived ' -- 2.25.1 ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-24 06:04 Emond Papegaaij <[email protected]> parent: Tatsuo Ishii <[email protected]> 0 siblings, 1 reply; 8+ messages in thread From: Emond Papegaaij @ 2025-07-24 06:04 UTC (permalink / raw) To: Tatsuo Ishii <[email protected]>; +Cc: [email protected] Hi, There's been some recent development on the side of pgjdbc as well: https://github.com/pgjdbc/pgjdbc/pull/3728 We are currently testing that fix. With that patch, pgjdbc will always send a sync first when in extended mode before sending a simple query. Do you think it's still a good idea to support mixing simple and extended mode in pgpool? It's difficult for me to assess the risk of your proposed patch. Maybe it's better to just not allow it if that makes it easier to implement? Ps. Sorry for the bad quoting. I'm writing this on my phone, as I'm still on vacation. Op do 24 jul 2025, 03:36 schreef Tatsuo Ishii <[email protected]>: > >> Hi Emond, > >> > >>>> > The problem however, is that previously the error was only > triggered when > >>>> > using autosave, which is not a common setup, but now the error is > >>>> triggered > >>>> > even when using default configuration. This makes it impossible to > >>>> combine > >>>> > PgJDBC 42.7.6 and up with pgpool-II. I do not know what the plans > are on > >>>> > this from the side of PostgreSQL and PgJDBC, but I just wanted to > raise > >>>> > some awareness on this issue here. IMHO PostgreSQL should either > >>>> explicitly > >>>> > allow this (in which case pgpool-II needs to be fixed) or explicitly > >>>> > disallow it (in which case PgJDBC needs to be fixed). > >>>> > >>>> As far as I know about PostgreSQL's side, Tom Lane said about this: > >>>> > https://www.postgresql.org/message-id/[email protected] > >>>> > >>>> > I think it's poor practice, at best. You should end the > >>>> > extended-protocol query cycle before invoking simple query. > >>>> > > >>>> > I'm disinclined to document, or make any promises about, > >>>> > what happens if you mix the protocols. > >>>> > >>>> In my understanding he does not say PostgreSQL explicitely allows this > >>>> (mixing extended and simple protocol message). > >>>> > >>> > >>> It's more or less allowed without any errors, but the expected > behavior is > >>> not clear nor is it documented. It seems libpq even throws an error on > the > >>> client side when you try to do this: > >>> > >>> Jelte Fennema-Nio in > >>> > https://www.postgresql.org/message-id/CAGECzQQ1hs2DU9pmQq18Y%3DqK4nZqhXDVg-sGEa5K01Lj4XMmxw%40mail.g... > >>>> I totally agree that it makes sense to throw an error in this case. > >>>> Libpq actually throws an error client side when a caller attempts to > >>>> do this, but this is something that should be checked server side, > >>>> given that the protocol docs specify this: > >>>> docs> At completion of each series of extended-query messages, the > >>>> frontend should issue a Sync message. > >>> > >>> > >>>> > The current situation > >>>> > is no good as we now simply cannot upgrade PgJDBC anymore (and the > same > >>>> > will be true for all other users of pgpool-II). > >>>> > >>>> Yeah. > >>>> > >>>> What I don't understand is, why PgJDBC decided to make it default > >>>> (sending simple protocol query after extended query protocl without > >>>> sync) even without autosave being set when they update PgJDBC to > >>>> 42.7.7. > >>>> > >>> > >>> It wasn't a real decision to make this happen more often. They fixed a > bug > >>> that caused many queries to be sent using the extended protocol even > when > >>> explicitly asked for a simple query: > >>> https://github.com/pgjdbc/pgjdbc/issues/3724#issuecomment-3051773696 > >>> > >>> In our case we got sporadic errors. These errors are very hard to > >>> reproduce. Some users were not able to login into our application at > all, > >>> others had no trouble whatsoever and some users got an error every > once in > >>> a while. Even when tests seem fine, you might get these errors later > on. > >>> None of our automated tests failed. On our testing environment we've > only > >>> seen the error twice in a period of 3 weeks. When we moved to > production, > >>> we triggered the error over 100 times in just 1 day! I would certainly > not > >>> recommend using PgJDBC 42.7.6 and up when using pgpool. > >> > >> I created a patch for pgpool to allow a simple query without ending a > >> sequence of extended query protocol message by a sync message. > >> > >> The idea is, within SimpleQuery() (a function responsible for > >> processing simple protocol query), check whether extended query > >> protocol ends. If not, call ProcessBackendReposnse() to process any > >> replies from backend such as parse complete, bind complete or command > >> complete and so. After this, SimpleQuery() can process simple query > >> as usual. I hope this solves the issue with PgJDBC 42.7.6. > > > > After thinking more, I concluded that it would be better to do the > > task in ProcessFrontendReposnse(), rather than in SimpleQuery() > > because we usually do not call ProcessBackendResponse() inside > > functions such as SimpleQuery() that are responsible for processing > > particular frontend message. > > > > Attached is the v2 patch. > > This is the v3 patch. Fix regression test 082.guard_against_bad_protocol. > Slightly modify commit message. > > Best regards, > -- > Tatsuo Ishii > SRA OSS K.K. > English: http://www.sraoss.co.jp/index_en/ > Japanese:http://www.sraoss.co.jp > ^ permalink raw reply [nested|flat] 8+ messages in thread
* Re: FATAL: simple query "BEGIN" arrived before ending an extended query message @ 2025-07-24 06:48 Tatsuo Ishii <[email protected]> parent: Emond Papegaaij <[email protected]> 0 siblings, 0 replies; 8+ messages in thread From: Tatsuo Ishii @ 2025-07-24 06:48 UTC (permalink / raw) To: [email protected]; +Cc: [email protected] > Hi, > > There's been some recent development on the side of pgjdbc as well: > https://github.com/pgjdbc/pgjdbc/pull/3728 > > We are currently testing that fix. With that patch, pgjdbc will always send > a sync first when in extended mode before sending a simple query. Ok. That's good. > Do you > think it's still a good idea to support mixing simple and extended mode in > pgpool? It's difficult for me to assess the risk of your proposed patch. I agree with your concern. Mixing simple and extended mode is a new code path in pgpool. Probably mixing simple and extended mode is also new code path in PostgreSQL backend. We can't ignore both risks. > Maybe it's better to just not allow it if that makes it easier to implement? I agree. I withdraw my patch. > Ps. Sorry for the bad quoting. No problem at all. > I'm writing this on my phone, as I'm still > on vacation. Sorry for bothering you vacation. Have a nice day! Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp ^ permalink raw reply [nested|flat] 8+ messages in thread
end of thread, other threads:[~2025-07-24 06:48 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-07-09 11:13 FATAL: simple query "BEGIN" arrived before ending an extended query message Emond Papegaaij <[email protected]> 2025-07-10 02:55 ` Tatsuo Ishii <[email protected]> 2025-07-10 06:34 ` Emond Papegaaij <[email protected]> 2025-07-21 04:12 ` Tatsuo Ishii <[email protected]> 2025-07-23 08:03 ` Tatsuo Ishii <[email protected]> 2025-07-24 01:36 ` Tatsuo Ishii <[email protected]> 2025-07-24 06:04 ` Emond Papegaaij <[email protected]> 2025-07-24 06:48 ` Tatsuo Ishii <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox