View Issue Details

IDProjectCategoryView StatusLast Update
0000444Pgpool-IIBugpublic2018-11-26 14:28
ReporterpensnarikAssigned Tohoshiai 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.7.6 
Target Version3.7.7Fixed in Version3.7.7 
Summary0000444: pgpool does not take into account parameters order in startup packet
DescriptionDuring comparison startup packets in function get_backend_connection() pgpool ignores parameter order. Since order is not defined in the PostgreSQL protocol it may cause problems for clients who implements the protocols by itself. For instance, golang PostgreSQL driver pq (https://github.com/lib/pq). As a consequence of this applications that use go pq library are not able to take advantage of pgpool connection caching.

Possible solutions are:

1) Sort or normalize parameters in startup packets during connection initialization
2) Compare startup packets regardless of parameter order

Link to the discussion in the mailing list: https://www.pgpool.net/pipermail/pgpool-general/2018-November/006327.html
Steps To Reproduce1) Write test application in golang which uses go pq library and connects to database with name different from template0, template1, postgres, regression.
2) Enable connection pooling in pg_pool
3) Observe in error log (with debug level > 0) messages "connection exists but startup packet contents is not identical" for each client connection
Tagsconnection cache, go, pgpool-II

Activities

hoshiai

2018-11-13 16:57

developer   ~0002244

I confirm this problem.

hoshiai

2018-11-19 11:13

developer   ~0002267

I reproduced it using simple go lang program.

hoshiai

2018-11-19 11:25

developer   ~0002268

Last edited: 2018-11-19 11:28

View 2 revisions

I created the patch for Pgpoo-II V3.7 for fixed this problem.
This patch sort the parameter in startup packet, when received startup packet.



test.go (990 bytes)
bug444_sort_startup_packet.patch (2,511 bytes)
 src/protocol/child.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/src/protocol/child.c b/src/protocol/child.c
index 652a62f..18ee044 100644
--- a/src/protocol/child.c
+++ b/src/protocol/child.c
@@ -511,6 +511,12 @@ static StartupPacket *read_startup_packet(POOL_CONNECTION *cp)
 	int protov;
 	int len;
 	char *p;
+	char **guc_options;
+	int opt_num = 0;
+	char *sp_sort;
+	char *tmpopt;
+	int i;
+	int j;
 
 	sp = (StartupPacket *)palloc0(sizeof(*sp));
 	enable_authentication_timeout();
@@ -537,7 +543,6 @@ static StartupPacket *read_startup_packet(POOL_CONNECTION *cp)
 	memcpy(&protov, sp->startup_packet, sizeof(protov));
 	sp->major = ntohl(protov)>>16;
 	sp->minor = ntohl(protov) & 0x0000ffff;
-	p = sp->startup_packet;
     cp->protoVersion = sp->major;
 
 	switch(sp->major)
@@ -554,6 +559,57 @@ static StartupPacket *read_startup_packet(POOL_CONNECTION *cp)
 			break;
 
 		case PROTO_MAJOR_V3: /* V3 */
+			/* copy startup_packet */
+			sp_sort = palloc0(len);
+			memcpy(sp_sort,sp->startup_packet,len);
+
+			p = sp_sort;
+			p += sizeof(int);	/* skip protocol version info */
+			/* count the number of options */
+			while(*p)
+			{
+				p += (strlen(p) + 1); /* skip option name */
+				p += (strlen(p) + 1); /* skip option value */
+				opt_num ++;
+			}
+			guc_options = (char **)palloc0(opt_num * sizeof(char *));
+			/* get guc_option name list */
+			p = sp_sort + sizeof(int);
+			for(i = 0; i < opt_num; i++)
+			{
+				guc_options[i] = p;
+				p += (strlen(p) + 1); /* skip option name */
+				p += (strlen(p) + 1); /* skip option value */
+			}
+			/* sort option name using quick sort */
+			for (i = 0; i < opt_num - 1 ; i++)
+			{
+				for (j = i + 1; j < opt_num; j++)
+				{
+					if (strcmp(guc_options[i], guc_options[j]) > 0)
+					{
+						tmpopt = guc_options[i];
+						guc_options[i] = guc_options[j];
+						guc_options[j] = tmpopt;
+					}
+				}
+			}
+
+			p = sp->startup_packet + sizeof(int);	/* skip protocol version info */
+			for (i = 0; i < opt_num; i++)
+			{
+				tmpopt = guc_options[i];
+				memcpy(p, tmpopt ,strlen(tmpopt) + 1); /* memcpy option name */
+				p += (strlen(tmpopt) + 1);
+				tmpopt += (strlen(tmpopt) + 1);
+				memcpy(p, tmpopt ,strlen(tmpopt) + 1); /* memcpy option value */
+				p += (strlen(tmpopt) + 1);
+			}
+
+			pfree(guc_options);
+			pfree(sp_sort);
+
+			p = sp->startup_packet;
 			p += sizeof(int);	/* skip protocol version info */
 
 			while(*p)

pensnarik

2018-11-19 11:36

reporter   ~0002269

Thank you for your patch, hoshiai. At what date/version it could be expected to see this code in the upstream?

hoshiai

2018-11-19 13:53

developer   ~0002271

Now I am testing this patch.
I am planing to include it in next minor version, So will commit it by then.
Next minor version release date is on Thursday, November 22nd, 2018.

hoshiai

2018-11-21 15:07

developer   ~0002276

I commited patch by 3.4 - 4.0 branches.
So this problem will fix by next minor version.

commit log:
https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=ffb998517bb91c512b77fbe1ca8ee331e492f131

hoshiai

2018-11-26 14:28

developer   ~0002284

Last week, new minor version relased , this problem fixed.

Issue History

Date Modified Username Field Change
2018-11-13 00:35 pensnarik New Issue
2018-11-13 00:35 pensnarik Tag Attached: connection cache
2018-11-13 00:35 pensnarik Tag Attached: pgpool-II
2018-11-13 00:35 pensnarik Tag Attached: go
2018-11-13 15:13 hoshiai Assigned To => hoshiai
2018-11-13 15:13 hoshiai Status new => assigned
2018-11-13 16:57 hoshiai Note Added: 0002244
2018-11-19 11:13 hoshiai Status assigned => confirmed
2018-11-19 11:13 hoshiai Note Added: 0002267
2018-11-19 11:25 hoshiai File Added: bug444_sort_startup_packet.patch
2018-11-19 11:25 hoshiai File Added: test.go
2018-11-19 11:25 hoshiai Note Added: 0002268
2018-11-19 11:28 hoshiai Note Edited: 0002268 View Revisions
2018-11-19 11:36 pensnarik Note Added: 0002269
2018-11-19 13:53 hoshiai Note Added: 0002271
2018-11-21 15:07 hoshiai Product Version => 3.7.6
2018-11-21 15:07 hoshiai Target Version => 3.7.7
2018-11-21 15:07 hoshiai Note Added: 0002276
2018-11-26 14:28 hoshiai Status confirmed => resolved
2018-11-26 14:28 hoshiai Resolution open => fixed
2018-11-26 14:28 hoshiai Fixed in Version => 3.7.7
2018-11-26 14:28 hoshiai Note Added: 0002284