Fix occasional 005.jdbc test failure.

The direct cause of the error is:
2023-02-22 08:51:47.705: PostgreSQL JDBC Driver pid 12420: LOG:  Parse: Error or notice message from backend: : DB node id: 0 backend pid: 12488 statement: "COMMIT" message: "prepared statement "S_1" already exists"

Actually the root of the error is this:
 2023-02-22 08:51:45.242: PostgreSQL JDBC Driver pid 12420: LOG:  pool_send_and_wait: Error or notice message from backend: : DB node id: 0 backend pid: 12488 statement: "DISCARD ALL" message: "DISCARD ALL cannot be executed within a pipeline"

"DISCARD ALL" was generated by pgpool (reset_query_list) to discard
some objects including prepared statements created in the
session. Since DISCARD ALL failed, the prepared statement S_1 was not
removed. Thus the next session failed because S_1 already existed.

To fix this, new global boolean flag reset_query_error is
introduced. The flag is set inside pool_send_and_wait() when a reset
query executed by SimpleQuery() results in ERROR. If the flag is true,
backend_cleanup() discards the backend connection so that any objects,
including named statement, corresponding to the session is discarded

For now I will push this to master branch only to see if the 005.jdbc
error gets fixed.  If ok, I will back patch to all supported branched.

Discussion: https://www.pgpool.net/pipermail/pgpool-hackers/2023-February/004293.html

diff --git a/src/context/pool_query_context.c b/src/context/pool_query_context.c
index 83c0a2b27d839783b2ab478ced05057955322c67..a9a2f670a13fabda6cc2b03a558b786673b98b04 100644 (file)
--- a/src/context/pool_query_context.c
+++ b/src/context/pool_query_context.c
@@ -929,8 +929,15 @@ pool_send_and_wait(POOL_QUERY_CONTEXT * query_context,
                 * Check if some error detected.  If so, emit log. This is useful when
                 * invalid encoding error occurs. In this case, PostgreSQL does not
                 * report what statement caused that error and make users confused.
+                * Also set reset_query_error to true in ERROR case. This does
+                * anything in normal query processing but when processing reset
+                * queries, this is important because it might mean DISCARD ALL
+                * command fails. If so, we need to discard the connection cache so
+                * that any session object (i.e. named statement) does not remain in
+                * the last session.
                 */
-               per_node_error_log(backend, i, string, "pool_send_and_wait: Error or notice message from backend: ", true);
+               if (per_node_error_log(backend, i, string, "pool_send_and_wait: Error or notice message from backend: ", true) == 'E')
+                       reset_query_error = true;
        }
 
        return POOL_CONTINUE;

diff --git a/src/include/pool.h b/src/include/pool.h
index 74f103233125f369c82e24819f8a9358c42aad73..50cf63d703dc9d683bebeef0a2bdf60b80b8d409 100644 (file)
--- a/src/include/pool.h
+++ b/src/include/pool.h
@@ -562,6 +562,7 @@ extern pid_t mypid;                         /* parent pid */
 extern pid_t myProcPid;                        /* process pid */
 extern ProcessType processType;
 extern ProcessState processState;
+extern bool    reset_query_error;      /* true if error occurs in reset queries */
 extern void set_application_name(ProcessType ptype);
 extern void set_application_name_with_string(char *string);
 extern void set_application_name_with_suffix(ProcessType ptype, int suffix);

diff --git a/src/include/protocol/pool_proto_modules.h b/src/include/protocol/pool_proto_modules.h
index 4ad7eade61ba45a533fbc4152eea76ad51676cf9..dc531af8757ee7bc38d72ef5601e88dff7c6e5c4 100644 (file)
--- a/src/include/protocol/pool_proto_modules.h
+++ b/src/include/protocol/pool_proto_modules.h
@@ -208,7 +208,7 @@ extern POOL_STATUS ErrorResponse(POOL_CONNECTION * frontend,
 extern void NoticeResponse(POOL_CONNECTION * frontend,
                           POOL_CONNECTION_POOL * backend);
 
-extern void per_node_error_log(POOL_CONNECTION_POOL * backend, int node_id,
-char *query, char *prefix, bool unread);
+extern char per_node_error_log(POOL_CONNECTION_POOL * backend, int node_id,
+                                                          char *query, char *prefix, bool unread);
 
 #endif

diff --git a/src/main/pool_globals.c b/src/main/pool_globals.c
index 25931a83c6b36f72f3fa0b0ef128032e3481d566..06f775ad9c3519fa09276907d18872944c19bb72 100644 (file)
--- a/src/main/pool_globals.c
+++ b/src/main/pool_globals.c
@@ -5,7 +5,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2021     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -29,6 +29,7 @@ pid_t         mypid;                          /* pgpool parent process id */
 pid_t          myProcPid;              /* process pid */
 ProcessType processType;
 ProcessState processState;
+bool           reset_query_error;      /* true if error returned from backend while processing reset queries */
 
 /*
  * Application name

diff --git a/src/protocol/child.c b/src/protocol/child.c
index f68c9eda8e507bd3bcda75b316141d18bc2d1a32..d64ff29981f9ccb11aaa35cca361ff9fb7b3bcef 100644 (file)
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -5,7 +5,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2022     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -542,8 +542,10 @@ backend_cleanup(POOL_CONNECTION * volatile *frontend, POOL_CONNECTION_POOL * vol
        if (cache_connection)
        {
                /*
-                * For those special databases, and when frontend client exits
-                * abnormally, we don't cache connection to backend.
+                * For those special databases, or when frontend client exits
+                * abnormally, we don't cache connection to backend.  Also we have to
+                * check whether reset query failed. If so, the existing connection to
+                * backend may not be used and we don't want to cache the connection.
                 */
                if ((sp &&
                         (!strcmp(sp->database, "template0") ||
@@ -552,7 +554,8 @@ backend_cleanup(POOL_CONNECTION * volatile *frontend, POOL_CONNECTION_POOL * vol
                          !strcmp(sp->database, "regression"))) ||
                        (*frontend != NULL &&
                         ((*frontend)->socket_state == POOL_SOCKET_EOF ||
-                         (*frontend)->socket_state == POOL_SOCKET_ERROR)))
+                         (*frontend)->socket_state == POOL_SOCKET_ERROR)) ||
+                       reset_query_error)
                        cache_connection = false;
        }
 

diff --git a/src/protocol/pool_process_query.c b/src/protocol/pool_process_query.c
index d4389d7b331dec63c0a72e38f62d93b6a3a172c8..d893eca743743bb0b139366a4533055787062df1 100644 (file)
--- a/src/protocol/pool_process_query.c
+++ b/src/protocol/pool_process_query.c
@@ -3,7 +3,7 @@
  * pgpool: a language independent connection pool server for PostgreSQL
  * written by Tatsuo Ishii
  *
- * Copyright (c) 2003-2022     PgPool Global Development Group
+ * Copyright (c) 2003-2023     PgPool Global Development Group
  *
  * Permission to use, copy, modify, and distribute this software and
  * its documentation for any purpose and without fee is hereby
@@ -133,6 +133,12 @@ pool_process_query(POOL_CONNECTION * frontend,
                memcached_connect();
        }
 
+       /*
+        * Reset error flag while processing reset queries.
+        * The flag is set to on inside pool_send_and_wait().
+        */
+       reset_query_error = false;
+
        for (;;)
        {
 

diff --git a/src/protocol/pool_proto_modules.c b/src/protocol/pool_proto_modules.c
index 0f9ce21c1e985158e21b51fd895a039248f10c5f..34edd509665e03d7a70af54275e0e41e8e38f8ba 100644 (file)
--- a/src/protocol/pool_proto_modules.c
+++ b/src/protocol/pool_proto_modules.c
@@ -3594,10 +3594,11 @@ per_node_statement_notice(POOL_CONNECTION_POOL * backend, int node_id, char *que
 }
 
 /*
- * Check kind and produce error message
+ * Check kind and produce error message.
+ * Return message kind.
  * All data read in this function is returned to stream.
  */
-void
+char
 per_node_error_log(POOL_CONNECTION_POOL * backend, int node_id, char *query, char *prefix, bool unread)
 {
        POOL_CONNECTION_POOL_SLOT *slot = backend->slots[node_id];
@@ -3609,7 +3610,7 @@ per_node_error_log(POOL_CONNECTION_POOL * backend, int node_id, char *query, cha
 
        if (kind != 'E' && kind != 'N')
        {
-               return;
+               return kind;
        }
 
        if (pool_extract_error_message(true, CONNECTION(backend, node_id), MAJOR(backend), unread, &message) == 1)
@@ -3619,6 +3620,7 @@ per_node_error_log(POOL_CONNECTION_POOL * backend, int node_id, char *query, cha
                                                prefix, node_id, ntohl(slot->pid), query, message)));
                pfree(message);
        }
+       return kind;
 }
 
 /*