On Mon, Apr 18, 2016 at 4:48 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> Another, perhaps, better idea is to add some more extra logic to
> pg_conn as follows:
> bool is_emergency;
> PGresult *emergency_result;
> bool is_emergency_consumed;
> So as when an OOM shows up, is_emergency is set to true. Then a first
> call of PQgetResult returns the PGresult with the OOM in
> emergency_result, setting is_emergency_consumed to true and switching
> is_emergency back to false. Then a second call to PQgetResult enforces
> the consumption of any results remaining without waiting for them, at
> the end returning NULL to the caller, resetting is_emergency_consumed
> to false. That's different compared to the extra failure
> PGASYNC_COPY_FAILED in the fact that it does not break PQisBusy().
So, in terms of code, it gives more or less the attached. I have
coupled the emergency_result with an enum flag emergency_status that
has 3 states:
- NONE, which is the default
- ENABLE, which is when an OOM or other error has been found in libpq.
Making the next call of PQgetResult return the emergency_result
- CONSUMED, set after PQgetResult is consuming emergency_result, to
return to caller NULL so as it can get out of any PQgetResult loop
expecting a result at the end of. Once NULL is returned, the flag is
switched back to NONE.
This works in the case of getCopyStart because the OOM failures are
happening before any messages are sent to server.
The checks for the emergency result are done in PQgetResult, so as any
upper-level routine gets the message. Tom, others, what do you think?
--
Michael
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 2621767..c5d7efd 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1689,6 +1689,30 @@ PQgetResult(PGconn *conn)
if (!conn)
return NULL;
+ if (!conn->emergency_result)
+ {
+ conn->emergency_result = PQmakeEmptyPGresult(conn, PGRES_FATAL_ERROR);
+ if (!conn->emergency_result)
+ return NULL;
+ conn->emergency_status = PGEMERGENCY_NONE;
+ }
+
+ /*
+ * Check for any error that could have happened with the client.
+ * Do this check before parsing any new commands.
+ */
+ switch (conn->emergency_status)
+ {
+ case PGEMERGENCY_NONE:
+ break;
+ case PGEMERGENCY_ENABLE:
+ conn->emergency_status = PGEMERGENCY_CONSUMED;
+ return conn->emergency_result;
+ case PGEMERGENCY_CONSUMED:
+ conn->emergency_status = PGEMERGENCY_NONE;
+ return NULL;
+ }
+
/* Parse any available data, if our state permits. */
parseInput(conn);
@@ -1698,6 +1722,22 @@ PQgetResult(PGconn *conn)
int flushResult;
/*
+ * Check for any error that could have happened with
+ * the client.
+ */
+ switch (conn->emergency_status)
+ {
+ case PGEMERGENCY_NONE:
+ break;
+ case PGEMERGENCY_ENABLE:
+ conn->emergency_status = PGEMERGENCY_CONSUMED;
+ return conn->emergency_result;
+ case PGEMERGENCY_CONSUMED:
+ conn->emergency_status = PGEMERGENCY_NONE;
+ return NULL;
+ }
+
+ /*
* If data remains unsent, send it. Else we might be waiting for the
* result of a command the backend hasn't even got yet.
*/
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 0b8c62f..6d99d12 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1453,7 +1453,7 @@ getCopyStart(PGconn *conn, ExecStatusType copytype)
result = PQmakeEmptyPGresult(conn, copytype);
if (!result)
- goto failure;
+ goto oom_failure;
if (pqGetc(&conn->copy_is_binary, conn))
goto failure;
@@ -1469,7 +1469,7 @@ getCopyStart(PGconn *conn, ExecStatusType copytype)
result->attDescs = (PGresAttDesc *)
pqResultAlloc(result, nfields * sizeof(PGresAttDesc), TRUE);
if (!result->attDescs)
- goto failure;
+ goto oom_failure;
MemSet(result->attDescs, 0, nfields * sizeof(PGresAttDesc));
}
@@ -1492,6 +1492,10 @@ getCopyStart(PGconn *conn, ExecStatusType copytype)
conn->result = result;
return 0;
+oom_failure:
+ conn->emergency_status = PGEMERGENCY_ENABLE;
+ printfPQExpBuffer(&conn->errorMessage,
+ libpq_gettext("out of memory\n"));
failure:
PQclear(result);
return EOF;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 1183323..e9dd167 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -223,6 +223,19 @@ typedef enum
PGASYNC_COPY_BOTH /* Copy In/Out data transfer in progress */
} PGAsyncStatusType;
+/*
+ * PGemergencyState defines the consumption status of emergency_result
+ * for clients-side error handling, particularly out-of-memory errors
+ * that could happen.
+ */
+typedef enum
+{
+ PGEMERGENCY_NONE, /* emergency result allocated, not needed */
+ PGEMERGENCY_ENABLE, /* ready to be consumed by client */
+ PGEMERGENCY_CONSUMED /* consumed by client, next call querying
+ * for a result will get NULL. */
+} PGemergencyState;
+
/* PGQueryClass tracks which query protocol we are now executing */
typedef enum
{
@@ -422,6 +435,10 @@ struct pg_conn
PGresult *result; /* result being constructed */
PGresult *next_result; /* next result (used in single-row mode) */
+ /* Status for client-side errors, likely out-of-memory issues */
+ PGemergencyState emergency_status;
+ PGresult *emergency_result;
+
/* Assorted state for SSL, GSS, etc */
#ifdef USE_SSL
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
