Re: Windows: Wrong error message at connection termination
Am 22.11.21 um 00:04 schrieb Tom Lane: Do we know that that actually happens in an arm's-length connection (ie two separate machines)? I wonder if the data loss is strictly an artifact of a localhost connection. There'd be a lot more pressure on them to make cross-machine TCP work per spec, one would think. But in any case, if we can avoid sending RST in this situation, it seems mostly moot for our usage. Sorry it took some days to get a setup to check this! The result is as expected: 1. Windows client to Linux server works without dropping the error message 2. Linux client to Windows server works without dropping the error message 3. Windows client to remote Windows server drops the error message, depending on the timing of the event loop In 1. the Linux server doesn't end the connection with a RST packet, so that the Windows client enqueues the error message properly and doesn't drop it. In 2. the Linux client doesn't care about the RST packet of the Windows server and properly enqueues and raises the error message. In 3. the combination of the bad RST behavior of client and server leads to data loss. It depends on the network timing. A delay of 0.5 ms in the event loop was enough in a localhost setup and as wall as in some LAN setup. On the contrary over some slower WLAN connection a delay of less than 15 ms did not loose data, but higher delays still did. The idea of running a second process, pass the socket handle to it, observe the parent process and close the socket when it exited, could work, but I guess it's overly complicated and creates more issues than it solves. Probably the same if the master process handles the socket closing. So I still think it's best to close the socket as proposed in the patch. -- Regards, Lars Kanis
Re: Windows: Wrong error message at connection termination
Am 18.11.21 um 03:04 schrieb Tom Lane: Thomas Munro writes: I realise now that the experiments we did a while back to try to understand this across a few different operating systems[2] had missed this subtlety, because that Python script had an explicit close() call, whereas PostgreSQL exits. It still revealed that the client isn't allowed to read any data after its write failed, which is a known source of error messages being eaten. Yeah. After re-reading that thread, I'm a bit confused about how to square the results we got then with Lars' report. The Windows documentation he pointed to does claim that the default behavior if you issue closesocket() is to do a "graceful close in the background", which one would think means allowing sent data to be received. That's not what we saw. It's possible that we would get different results if we re-tested with a scenario where the client doesn't attempt to send data after the server-side close; but I'm not sure how much it's worth to improve that case if the other case still fails hard. Form my experimentation the Winsock implementation has the two issues which I explained. First it drops all received but not yet retrieved data as soon as it receives a RST packet. And secondly it always sends a RST packet on every socket, that wasn't send-closed at process termination, regardless if there is any pending data. Sending data to a socket, that was already closed from the other side is only one way to trigger a RST packet, but closing a socket with l_linger=0 is another way and process termination is the third. They all can lead to data loss on the receiver side, presumably because of the RST flag. An alternative to closesocket() is shutdown(sock, SD_SEND). It doesn't free the socket resource, but leads to a graceful shutdown. However the FIN packet is send when the shutdown() or closesocket() function is called and that's still short before the process terminates. I did some more testing with different linger options, but it didn't change the behavior substantial. So I didn't find any way to close the socket with a FIN packet at the point in time of the process termination. The other way around would be to make sure on the client side, that the last message is retrieved before the RST packet arrives, so that no data is lost. This works mostly well through the sync API of libpq, but with the async API the trigger for data reception is outside of the scope of libpq, so that there's no way to ensure recv() is called quick enough, after the data was received but before RST arrives. On a local client+server combination there is only a gap of 0.5 milliseconds or so. I also didn't find a way to retrieve the enqueued data after RST arrived. Maybe there's a nasty hack to retrieve the data afterwards, but I didn't dig into assembly code and memory layout of Winsock internals. In any case, our previous results definitely show that issuing an explicit close() is no panacea. I don't fully understand the issue with closing the socket before process termination. Sure, it can be a valuable information that the corresponding backend process has definitely terminated. At least in the context of regression testing or so. But I think that loosing messages from the backend is way more critical than a non-sync process termination. Do I miss something? -- Regards, Lars Kanis
Windows: Wrong error message at connection termination
Dear hackers, I lately had a hard time to find the root cause for some wired behavior with the async API of libpq when running client and server on Windows. When the connection aborts with an error - most notably with an error at the connection setup - it sometimes fails with a wrong error message: Instead of: connection to server at "::1", port 5433 failed: FATAL: role "a" does not exist it fails with: connection to server at "::1", port 5433 failed: server closed the connection unexpectedly I found out, that the recv() function of the Winsock API has some wired behavior. If the connection receives a TCP RST flag, recv() immediately returns -1, regardless if all previous data has been retrieved. So when the connection is closed hard, the behavior is timing dependent on the client side. It may drop the last packet or it delivers it to libpq, if libpq calls recv() quick enough. This behavior is described at closesocket() here: https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-closesocket This is called a hard or abortive close, because the socket's virtual circuit is reset immediately, and any unsent data is lost. On Windows, any *recv* call on the remote side of the circuit will fail with WSAECONNRESET <https://docs.microsoft.com/en-us/windows/desktop/WinSock/windows-sockets-error-codes-2>. Unfortunately each connection is closed hard by a Windows PostgreSQL server with TCP flag RST. That in turn is another Winsock API behavior, that is that every socket, that wasn't closed by the application is closed hard with the RST flag at process termination. I didn't find any official documentation about this behavior. Explicit closing the socket before process termination leads to a graceful close even on Windows. That is done by the attached patch. I think delivering the correct error message to the user is much more important that closing the process in sync with the socket. Some background: I'm the maintainer of ruby-pg, the PostgreSQL client library for ruby. The next version of ruby-pg will switch to the async API for connection setup. Using this API changes the timing of socket operations and therefore often leads to the above wrong message. Previous versions made use of the sync API, which usually doesn't suffer from this issue. The original issue is here: https://github.com/ged/ruby-pg/issues/404 -- Kind Regards Lars Kanis From 079c3dce7c4580a797267b6e42b33399606b4f9d Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Wed, 17 Nov 2021 21:04:45 +0100 Subject: [PATCH] Windows: Gracefully close the socket on process exit This is to avoid a hard close of the socket with tcp RST flag, which in turn can lead to a dropped error message on a Windows client. --- src/backend/libpq/pqcomm.c | 12 1 file changed, 12 insertions(+) diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 9ebba025c..7a81fa913 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -286,6 +286,18 @@ socket_close(int code, Datum arg) * We do set sock to PGINVALID_SOCKET to prevent any further I/O, * though. */ + +#ifdef WIN32 + /* + * On Windows we still do an explicit close here, because Windows + * closes sockets with RST flag instead of FIN at process termination. + * On the client side it then leads to a WSAECONNRESET error, which + * can (depending on the timing) drop the last error message, leading + * to a valuable information loss for the user. + */ + closesocket(MyProcPort->sock); +#endif + MyProcPort->sock = PGINVALID_SOCKET; } } -- 2.32.0
Re: libpq: Fix wrong connection status on invalid "connect_timeout"
Am 18.10.19 um 05:06 schrieb Michael Paquier: > So attached is a patch to skip trailing whitespaces as well, > which also fixes the issue with ECPG. I have refactored the parsing > logic a bit while on it. The comment at the top of parse_int_param() > needs to be reworked a bit more. I tested this and it looks good to me. Maybe you could omit some redundant 'end' checks, as in the attached patch. Or was your intention to verify non-NULL 'end'? > Perhaps we could add directly regression > tests for libpq. I'll start a new thread about that once we are done > here, the topic is larger. We have around 650 tests on ruby-pg to ensure everything runs as expected and I always wondered how the API of libpq is being verified. -- Kind Regards, Lars Kanis diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index f91f0f2..0eeabb8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1687,7 +1687,7 @@ useKeepalives(PGconn *conn) /* * Parse and try to interpret "value" as an integer value, and if successful, * store it in *result, complaining if there is any trailing garbage or an - * overflow. + * overflow. This allows any number of leading and trailing whitespaces. */ static bool parse_int_param(const char *value, int *result, PGconn *conn, @@ -1698,14 +1698,31 @@ parse_int_param(const char *value, int *result, PGconn *conn, *result = 0; + /* strtol(3) skips leading whitespaces */ errno = 0; numval = strtol(value, , 10); - if (errno == 0 && *end == '\0' && numval == (int) numval) - { - *result = numval; - return true; - } + /* + * If no progress was done during the parsing or an error happened, fail. + * This tests properly for overflows of the result. + */ + if (value == end || errno != 0 || numval != (int) numval) + goto error; + + /* + * Skip any trailing whitespace; if anything but whitespace remains before + * the terminating character, fail + */ + while (*end != '\0' && isspace((unsigned char) *end)) + end++; + + if (*end != '\0') + goto error; + + *result = numval; + return true; + +error: appendPQExpBuffer(>errorMessage, libpq_gettext("invalid integer value \"%s\" for connection option \"%s\"\n"), value, context); @@ -2008,7 +2025,11 @@ connectDBComplete(PGconn *conn) { if (!parse_int_param(conn->connect_timeout, , conn, "connect_timeout")) + { + /* mark the connection as bad to report the parsing failure */ + conn->status = CONNECTION_BAD; return 0; + } if (timeout > 0) {
Re: libpq: Fix wrong connection status on invalid "connect_timeout"
Am 18.10.19 um 05:06 schrieb Michael Paquier: > So attached is a patch to skip trailing whitespaces as well, > which also fixes the issue with ECPG. I have refactored the parsing > logic a bit while on it. The comment at the top of parse_int_param() > needs to be reworked a bit more. I tested this and it looks good to me. Maybe you could omit some redundant 'end' checks, as in the attached patch. Or was your intention to verify non-NULL 'end'? > Perhaps we could add directly regression > tests for libpq. I'll start a new thread about that once we are done > here, the topic is larger. We have around 650 tests on ruby-pg to ensure everything runs as expected and I always wondered how the API of libpq is being verified. -- Kind Regards, Lars Kanis diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index f91f0f2..0eeabb8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1687,7 +1687,7 @@ useKeepalives(PGconn *conn) /* * Parse and try to interpret "value" as an integer value, and if successful, * store it in *result, complaining if there is any trailing garbage or an - * overflow. + * overflow. This allows any number of leading and trailing whitespaces. */ static bool parse_int_param(const char *value, int *result, PGconn *conn, @@ -1698,14 +1698,31 @@ parse_int_param(const char *value, int *result, PGconn *conn, *result = 0; + /* strtol(3) skips leading whitespaces */ errno = 0; numval = strtol(value, , 10); - if (errno == 0 && *end == '\0' && numval == (int) numval) - { - *result = numval; - return true; - } + /* + * If no progress was done during the parsing or an error happened, fail. + * This tests properly for overflows of the result. + */ + if (value == end || errno != 0 || numval != (int) numval) + goto error; + + /* + * Skip any trailing whitespace; if anything but whitespace remains before + * the terminating character, fail + */ + while (*end != '\0' && isspace((unsigned char) *end)) + end++; + + if (*end != '\0') + goto error; + + *result = numval; + return true; + +error: appendPQExpBuffer(>errorMessage, libpq_gettext("invalid integer value \"%s\" for connection option \"%s\"\n"), value, context); @@ -2008,7 +2025,11 @@ connectDBComplete(PGconn *conn) { if (!parse_int_param(conn->connect_timeout, , conn, "connect_timeout")) + { + /* mark the connection as bad to report the parsing failure */ + conn->status = CONNECTION_BAD; return 0; + } if (timeout > 0) {
Re: libpq: Fix wrong connection status on invalid "connect_timeout"
Am 18.10.19 um 05:06 schrieb Michael Paquier: > So attached is a patch to skip trailing whitespaces as well, > which also fixes the issue with ECPG. I have refactored the parsing > logic a bit while on it. The comment at the top of parse_int_param() > needs to be reworked a bit more. I tested this and it looks good to me. Maybe you could omit some redundant 'end' checks, as in the attached patch. Or was your intention to verify non-NULL 'end'? > Perhaps we could add directly regression > tests for libpq. I'll start a new thread about that once we are done > here, the topic is larger. We have around 650 tests on ruby-pg to ensure everything runs as expected and I always wondered how the API of libpq is being verified. -- Kind Regards, Lars Kanis diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index f91f0f2..0eeabb8 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1687,7 +1687,7 @@ useKeepalives(PGconn *conn) /* * Parse and try to interpret "value" as an integer value, and if successful, * store it in *result, complaining if there is any trailing garbage or an - * overflow. + * overflow. This allows any number of leading and trailing whitespaces. */ static bool parse_int_param(const char *value, int *result, PGconn *conn, @@ -1698,14 +1698,31 @@ parse_int_param(const char *value, int *result, PGconn *conn, *result = 0; + /* strtol(3) skips leading whitespaces */ errno = 0; numval = strtol(value, , 10); - if (errno == 0 && *end == '\0' && numval == (int) numval) - { - *result = numval; - return true; - } + /* + * If no progress was done during the parsing or an error happened, fail. + * This tests properly for overflows of the result. + */ + if (value == end || errno != 0 || numval != (int) numval) + goto error; + + /* + * Skip any trailing whitespace; if anything but whitespace remains before + * the terminating character, fail + */ + while (*end != '\0' && isspace((unsigned char) *end)) + end++; + + if (*end != '\0') + goto error; + + *result = numval; + return true; + +error: appendPQExpBuffer(>errorMessage, libpq_gettext("invalid integer value \"%s\" for connection option \"%s\"\n"), value, context); @@ -2008,7 +2025,11 @@ connectDBComplete(PGconn *conn) { if (!parse_int_param(conn->connect_timeout, , conn, "connect_timeout")) + { + /* mark the connection as bad to report the parsing failure */ + conn->status = CONNECTION_BAD; return 0; + } if (timeout > 0) {
Re: libpq: Fix wrong connection status on invalid "connect_timeout"
I verified that all other integer parameters properly set CONNECTION_BAD in case of invalid values. These are: * port * keepalives_idle * keepalives_interval * keepalives_count * tcp_user_timeout That's why I changed connectDBComplete() only, instead of setting the status directly in parse_int_param(). -- Kind Regards, Lars Kanis Am 17.10.19 um 20:04 schrieb Lars Kanis: > Greetings, > > libpq since PostgreSQL-12 has stricter checks for integer values in > connection parameters. They were introduced by commit > https://github.com/postgres/postgres/commit/e7a2217978d9cbb2149bfcb4ef1e45716cfcbefb > . > > However in case of "connect_timeout" such an invalid integer value leads > to a connection status other than CONNECTION_OK or CONNECTION_BAD. The > wrong parameter is therefore not properly reported to user space. This > patch fixes this by explicit setting CONNECTION_BAD. > > The issue was raised on ruby-pg: https://github.com/ged/ruby-pg/issues/302 > > It originally came up at Heroku: > https://github.com/heroku/stack-images/issues/147 > -- -- Kind Regards, Lars Kanis
libpq: Fix wrong connection status on invalid "connect_timeout"
Greetings, libpq since PostgreSQL-12 has stricter checks for integer values in connection parameters. They were introduced by commit https://github.com/postgres/postgres/commit/e7a2217978d9cbb2149bfcb4ef1e45716cfcbefb . However in case of "connect_timeout" such an invalid integer value leads to a connection status other than CONNECTION_OK or CONNECTION_BAD. The wrong parameter is therefore not properly reported to user space. This patch fixes this by explicit setting CONNECTION_BAD. The issue was raised on ruby-pg: https://github.com/ged/ruby-pg/issues/302 It originally came up at Heroku: https://github.com/heroku/stack-images/issues/147 -- Kind Regards, Lars Kanis From f0c0dc3511c66a1e0fe0e4cc7ad2995f5f40bd7c Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Thu, 17 Oct 2019 19:37:51 +0200 Subject: [PATCH] Fix wrong connection status on invalid "connect_timeout" Commit e7a2217978d9cbb2149bfcb4ef1e45716cfcbefb introduced stricter checks for integer values in connection parameters. However in case of "connect_timeout" such invalid integer values leaded to a connection status other than CONNECTION_OK or CONNECTION_BAD. This patch explicit sets CONNECTION_BAD. Signed-off-by: Lars Kanis --- src/interfaces/libpq/fe-connect.c | 4 1 file changed, 4 insertions(+) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index f91f0f2..44d927e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2008,7 +2008,11 @@ connectDBComplete(PGconn *conn) { if (!parse_int_param(conn->connect_timeout, , conn, "connect_timeout")) + { + /* invalid integer */ + conn->status = CONNECTION_BAD; return 0; + } if (timeout > 0) { -- 2.17.1
[PATCH] Fix docs to JOHAB encoding on server side
The current documentation is inconsistent about the JOHAB character encoding on server side. While the first table says that it is not possible to use JOHAB on server side, it is still listed in table 23.2 as a server character set: https://www.postgresql.org/docs/devel/static/multibyte.html#MULTIBYTE-CHARSET-SUPPORTED -- Kind Regards, Lars From 65d60fc66cf62af9ea2d570590a541fbdf397767 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sat, 1 Sep 2018 18:03:14 +0200 Subject: [PATCH] Fix docs to JOHAB encoding on server side This is probably left over from commit 6041b92238897b06fe7bbe229a6e99f80121fa4a . --- doc/src/sgml/charset.sgml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/src/sgml/charset.sgml b/doc/src/sgml/charset.sgml index dc3fd34a62..452e625e01 100644 --- a/doc/src/sgml/charset.sgml +++ b/doc/src/sgml/charset.sgml @@ -1538,8 +1538,7 @@ $ psql -l JOHAB - JOHAB, - UTF8 + not supported as a server encoding -- 2.17.1
Re: Retrieve memory size allocated by libpq
Attached is an updated patch of PQresultMemsize(). The implementation is unchanged, but I added SGML documentation about this new function. I'd be pleased about comments to adding this to libpq. -- Kind Regards, Lars From 766a7f2381ae6c9d442a7359cabc58515186f8c4 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sat, 23 Jun 2018 19:34:11 +0200 Subject: [PATCH] libpq: Add function PQresultMemsize() This function retrieves the number of bytes allocated for a given result. That can be used to instruct the GC about the memory consumed behind a wrapping object and for diagnosing memory consumption. This is an alternative approach to customizable malloc/realloc/free functions as discussed here: https://www.postgresql.org/message-id/flat/20170828172834.GA71455%40TC.local#20170828172834.GA71455@TC.local --- doc/src/sgml/libpq.sgml | 28 src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-exec.c | 14 +- src/interfaces/libpq/libpq-fe.h | 1 + src/interfaces/libpq/libpq-int.h | 2 ++ 5 files changed, 45 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index d67212b831..c573c79ae3 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -6384,6 +6384,34 @@ void *PQresultAlloc(PGresult *res, size_t nBytes); + + + PQresultMemsize + + PQresultMemsize + + + + + + Retrieves the number of bytes allocated for a PGresult object. + +size_t PQresultMemsize(const PGresult *res); + + + + + The number of bytes includes the memory allocated for the PGresult itself, + memory to store data from the server, required internal metadata of a + PGresult object and data allocated by PQresultAlloc. + That is to say all memory which gets freed by PQclear. + + This information can be used for diagnosing memory consumption and to instruct + a garbage collector about the memory consumed behind a wrapping object. + + + + PQlibVersion diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index d6a38d0df8..0b50dddbb7 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -172,3 +172,4 @@ PQsslAttribute169 PQsetErrorContextVisibility 170 PQresultVerboseErrorMessage 171 PQencryptPasswordConn 172 +PQresultMemsize 173 diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 4c0114c514..064c7a693c 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -166,6 +166,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->memsize = sizeof(PGresult); if (conn) { @@ -215,6 +216,12 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) return result; } +size_t +PQresultMemsize(const PGresult *res) +{ + return res->memsize; +} + /* * PQsetResultAttrs * @@ -567,9 +574,11 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary) */ if (nBytes >= PGRESULT_SEP_ALLOC_THRESHOLD) { - block = (PGresult_data *) malloc(nBytes + PGRESULT_BLOCK_OVERHEAD); + size_t alloc_size = nBytes + PGRESULT_BLOCK_OVERHEAD; + block = (PGresult_data *) malloc(alloc_size); if (!block) return NULL; + res->memsize += alloc_size; space = block->space + PGRESULT_BLOCK_OVERHEAD; if (res->curBlock) { @@ -594,6 +603,7 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary) block = (PGresult_data *) malloc(PGRESULT_DATA_BLOCKSIZE); if (!block) return NULL; + res->memsize += PGRESULT_DATA_BLOCKSIZE; block->next = res->curBlock; res->curBlock = block; if (isBinary) @@ -711,6 +721,7 @@ PQclear(PGresult *res) res->errFields = NULL; res->events = NULL; res->nEvents = 0; + res->memsize = 0; /* res->curBlock was zeroed out earlier */ /* Free the PGresult structure itself */ @@ -927,6 +938,7 @@ pqAddTuple(PGresult *res, PGresAttValue *tup, const char **errmsgp) realloc(res->tuples, newSize * sizeof(PGresAttValue *)); if (!newTuples) return false; /* malloc or realloc failed */ + res->memsize += (newSize - res->tupArrSize) * sizeof(PGresAttValue *); res->tupArrSize = newSize; res->tuples = newTuples; } diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index ed9c806861..4fd9a4fcda 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -491,6 +491,7 @@ extern int PQgetlength(const PGresult *res, int tup_num, int field_num); extern int PQgetisnull(const PGresult *res, int tup_num, int field_num); extern int PQnparams(const PGresult *res); extern Oid PQparamtype(const PGresult *res, int param_num); +extern size_t PQresultMemsize(const PGresult *res); /* Describe prepared statements an
Retrieve memory size allocated by libpq
Hello, I would like to be able to retrieve the size of memory internally allocated by libpq for a result. The reason is that we have a Ruby wrapper that exposes libpq in Ruby. The problem is that Ruby's GC doesn't know how much memory has been allocated by libpq, so no pressure is applied to the GC when it should be. With this function we could instruct the GC about the memory usage associated to each result object. This issue has already been discussed in the following thread, with the request to use custom malloc/realloc/free functions: https://www.postgresql.org/message-id/flat/20170828172834.GA71455%40TC.local#20170828172834.GA71455@TC.local Retrieving the allocated memory size is another approach to solve the same base issue. However since the relation between memory consumption and the particular result object is maintained, it can additionally be used to provide diagnostic information to each object. What do you think about adding such a function? -- Kind Regards, Lars From d3ac8089a1b8c26d29d8d8e93c48a892cec75e53 Mon Sep 17 00:00:00 2001 From: Lars Kanis Date: Sat, 23 Jun 2018 19:34:11 +0200 Subject: [PATCH] libpq: Add function PQresultMemsize() This function retrieves the number of bytes allocated for a given result. That can be used to instruct the GC about the memory consumed behind a wrapping object and for diagnosing memory consumtion. This is an alternative approach to customizable malloc/realloc/free functions as discussed here: https://www.postgresql.org/message-id/flat/20170828172834.GA71455%40TC.local#20170828172834.GA71455@TC.local --- src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-exec.c | 14 +- src/interfaces/libpq/libpq-fe.h | 1 + src/interfaces/libpq/libpq-int.h | 2 ++ 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index d6a38d0df8..0b50dddbb7 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -172,3 +172,4 @@ PQsslAttribute169 PQsetErrorContextVisibility 170 PQresultVerboseErrorMessage 171 PQencryptPasswordConn 172 +PQresultMemsize 173 diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 4c0114c514..064c7a693c 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -166,6 +166,7 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) result->curBlock = NULL; result->curOffset = 0; result->spaceLeft = 0; + result->memsize = sizeof(PGresult); if (conn) { @@ -215,6 +216,12 @@ PQmakeEmptyPGresult(PGconn *conn, ExecStatusType status) return result; } +size_t +PQresultMemsize(const PGresult *res) +{ + return res->memsize; +} + /* * PQsetResultAttrs * @@ -567,9 +574,11 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary) */ if (nBytes >= PGRESULT_SEP_ALLOC_THRESHOLD) { - block = (PGresult_data *) malloc(nBytes + PGRESULT_BLOCK_OVERHEAD); + size_t alloc_size = nBytes + PGRESULT_BLOCK_OVERHEAD; + block = (PGresult_data *) malloc(alloc_size); if (!block) return NULL; + res->memsize += alloc_size; space = block->space + PGRESULT_BLOCK_OVERHEAD; if (res->curBlock) { @@ -594,6 +603,7 @@ pqResultAlloc(PGresult *res, size_t nBytes, bool isBinary) block = (PGresult_data *) malloc(PGRESULT_DATA_BLOCKSIZE); if (!block) return NULL; + res->memsize += PGRESULT_DATA_BLOCKSIZE; block->next = res->curBlock; res->curBlock = block; if (isBinary) @@ -711,6 +721,7 @@ PQclear(PGresult *res) res->errFields = NULL; res->events = NULL; res->nEvents = 0; + res->memsize = 0; /* res->curBlock was zeroed out earlier */ /* Free the PGresult structure itself */ @@ -927,6 +938,7 @@ pqAddTuple(PGresult *res, PGresAttValue *tup, const char **errmsgp) realloc(res->tuples, newSize * sizeof(PGresAttValue *)); if (!newTuples) return false; /* malloc or realloc failed */ + res->memsize += (newSize - res->tupArrSize) * sizeof(PGresAttValue *); res->tupArrSize = newSize; res->tuples = newTuples; } diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index ed9c806861..4fd9a4fcda 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -491,6 +491,7 @@ extern int PQgetlength(const PGresult *res, int tup_num, int field_num); extern int PQgetisnull(const PGresult *res, int tup_num, int field_num); extern int PQnparams(const PGresult *res); extern Oid PQparamtype(const PGresult *res, int param_num); +extern size_t PQresultMemsize(const PGresult *res); /* Describe prepared statements and portals */ extern PGresult *PQdescribePrepared(PGconn *conn, const char *stmt); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 9a586ff25a..37c9c3853d 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/l