Re: Windows: Wrong error message at connection termination

2021-11-27 Thread Lars Kanis

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

2021-11-21 Thread Lars Kanis

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

2021-11-17 Thread Lars Kanis

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"

2019-10-18 Thread Lars Kanis
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"

2019-10-18 Thread Lars Kanis
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"

2019-10-18 Thread Lars Kanis
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"

2019-10-17 Thread Lars Kanis
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"

2019-10-17 Thread 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

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

2018-09-01 Thread Lars Kanis
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

2018-07-10 Thread Lars Kanis
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

2018-06-23 Thread Lars Kanis
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