postgres_fdw: Report warning when timeout expires while getting query result.

When aborting remote transaction or sending cancel request to a remote server,
postgres_fdw calls pgfdw_get_cleanup_result() to wait for the result of
transaction abort query or cancel request to arrive. It fails to get the result
if the timeout expires or a connection trouble happens.

Previously postgres_fdw reported no warning message even when the timeout
expired or a connection trouble happened in pgfdw_get_cleanup_result().
This could make the troubleshooting harder when such an event occurred.

This commit makes pgfdw_get_cleanup_result() tell its caller what trouble
(timeout or connection error) occurred, on failure, and also makes its caller
report the proper warning message based on that information.

Author: Fujii Masao
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/15aa988c-722e-ad3e-c936-4420c5b2bfea@oss.nttdata.com

diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 5c0137220a9b6aab80a39e45f3d7414840ca60c2..6bac4ad23eb14bb94668c0ca686bb5b396a1a9bd 100644 (file)
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -104,7 +104,7 @@ static bool pgfdw_cancel_query(PGconn *conn);
 static bool pgfdw_exec_cleanup_query(PGconn *conn, const char *query,
                                                                         bool ignore_errors);
 static bool pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime,
-                                                                        PGresult **result);
+                                                                        PGresult **result, bool *timed_out);
 static void pgfdw_abort_cleanup(ConnCacheEntry *entry, const char *sql,
                                                                bool toplevel);
 static bool UserMappingPasswordRequired(UserMapping *user);
@@ -1154,6 +1154,7 @@ pgfdw_cancel_query(PGconn *conn)
        char            errbuf[256];
        PGresult   *result = NULL;
        TimestampTz endtime;
+       bool            timed_out;
 
        /*
         * If it takes too long to cancel the query and discard the result, assume
@@ -1180,8 +1181,19 @@ pgfdw_cancel_query(PGconn *conn)
        }
 
        /* Get and discard the result of the query. */
-       if (pgfdw_get_cleanup_result(conn, endtime, &result))
+       if (pgfdw_get_cleanup_result(conn, endtime, &result, &timed_out))
+       {
+               if (timed_out)
+                       ereport(WARNING,
+                                       (errmsg("could not get result of cancel request due to timeout")));
+               else
+                       ereport(WARNING,
+                                       (errcode(ERRCODE_CONNECTION_FAILURE),
+                                        errmsg("could not get result of cancel request: %s",
+                                                       pchomp(PQerrorMessage(conn)))));
+
                return false;
+       }
        PQclear(result);
 
        return true;
@@ -1204,6 +1216,7 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
 {
        PGresult   *result = NULL;
        TimestampTz endtime;
+       bool            timed_out;
 
        /*
         * If it takes too long to execute a cleanup query, assume the connection
@@ -1224,8 +1237,17 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
        }
 
        /* Get the result of the query. */
-       if (pgfdw_get_cleanup_result(conn, endtime, &result))
+       if (pgfdw_get_cleanup_result(conn, endtime, &result, &timed_out))
+       {
+               if (timed_out)
+                       ereport(WARNING,
+                                       (errmsg("could not get query result due to timeout"),
+                                        query ? errcontext("remote SQL command: %s", query) : 0));
+               else
+                       pgfdw_report_error(WARNING, NULL, conn, false, query);
+
                return false;
+       }
 
        /* Issue a warning if not successful. */
        if (PQresultStatus(result) != PGRES_COMMAND_OK)
@@ -1245,15 +1267,19 @@ pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors)
  * side back to the appropriate state.
  *
  * endtime is the time at which we should give up and assume the remote
- * side is dead.  Returns true if the timeout expired, otherwise false.
- * Sets *result except in case of a timeout.
+ * side is dead.  Returns true if the timeout expired or connection trouble
+ * occurred, false otherwise.  Sets *result except in case of a timeout.
+ * Sets timed_out to true only when the timeout expired.
  */
 static bool
-pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result)
+pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result,
+                                                bool *timed_out)
 {
-       volatile bool timed_out = false;
+       volatile bool failed = false;
        PGresult   *volatile last_res = NULL;
 
+       *timed_out = false;
+
        /* In what follows, do not leak any PGresults on an error. */
        PG_TRY();
        {
@@ -1271,7 +1297,8 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result)
                                cur_timeout = TimestampDifferenceMilliseconds(now, endtime);
                                if (cur_timeout <= 0)
                                {
-                                       timed_out = true;
+                                       *timed_out = true;
+                                       failed = true;
                                        goto exit;
                                }
 
@@ -1290,8 +1317,8 @@ pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result)
                                {
                                        if (!PQconsumeInput(conn))
                                        {
-                                               /* connection trouble; treat the same as a timeout */
-                                               timed_out = true;
+                                               /* connection trouble */
+                                               failed = true;
                                                goto exit;
                                        }
                                }
@@ -1313,11 +1340,11 @@ exit:   ;
        }
        PG_END_TRY();
 
-       if (timed_out)
+       if (failed)
                PQclear(last_res);
        else
                *result = last_res;
-       return timed_out;
+       return failed;
 }
 
 /*