Re: Multiple hosts in connection string failed to failover in non-hot standby mode
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Hubert Zhang <zhubert@vmware.com>, tsunakawa.takay@fujitsu.com, pgsql-hackers@postgresql.org, Andreas Seltenreich <seltenreich@gmx.de>, Bruce Momjian <bruce@momjian.us>
Date: 2021-06-09T01:52:10Z
Lists: pgsql-hackers
On Tue, Jun 08, 2021 at 11:21:34AM -0400, Tom Lane wrote: > IIUC, what you are proposing to do is replace pgwin32_setenv with > src/port/setenv.c. I don't think that's an improvement. setenv.c > leaks memory on repeat calls, because it cannot know what > pgwin32_setenv knows about how putenv works on that platform. Is gaur the only animal that needs this file, by the way? > (1) allow just this one ECPG test to include port.h (or > probably c.h). However, there's a whole other can of worms > there, which is that I wonder if we aren't doing it wrong > on the Unix side by linking libpgport when we shouldn't. > We've not been bit by that yet, but I wonder if it isn't > just a matter of time. The MSVC build, by not linking > those libs in the first place, is really doing this the > correct way. I don't really want to include this stuff in the ECPG tests just to bypass an environment configuration. > (2) Let pg_regress_ecpg.c pass down the environment setting. > > (3) Don't try to use the environment variable for this > purpose. I'd originally tried to change test5.pgc to just > specify gssmode=disable in-line, but that only works > nicely for one of the two failing cases. The other one > is testing the case of a completely defaulted connection > target, so there's no place to add an option without > breaking the only unique aspect of that test case. > (2) is starting to seem attractive now that we've seen > the downsides of (1) and (3). FWIW, I'd be rather in favor of doing (3) because this remains simple just to take care of an edge case, even if that partially breaks the promise to rely on a default connection. (4) would be to revisit the decision to make libpq report all the errors stored in its stack with multiple attempts. That would bring back the buildfarm to green at least, and we still need to take a decision about that for 14 anyway as it involves a compatibility breakage. But I agree that we also should do something for 12~ for those tests. > (BTW, I just noticed that regress.c is unsetenv'ing the > SSL connection environment variables, but not the GSS ones. > Seens like that needs work similar to 8279f68a1.) Yes, I saw that I was able to break things is many fancy ways when working on 8279f68a1, but the list of parameters to reset needs to diverge a bit compared to the TAP tests. -- Michael
Commits
-
Clear conn->errorMessage at successful completion of PQconnectdb().
- 896a0c44f93b 14.0 landed
- 138531f1bbc3 15.0 landed
-
Avoid ECPG test failures in some GSS-capable environments.
- 9bb5eecce645 14.0 landed
-
Try next host after a "cannot connect now" failure.
- c1d589571c49 14.0 landed
-
Uniformly identify the target host in libpq connection failure reports.
- 52a10224e3cc 14.0 landed
-
Allow pg_regress.c wrappers to postprocess test result files.
- 800d93f314b0 14.0 landed
-
In libpq, always append new error messages to conn->errorMessage.
- ffa2e4670123 14.0 landed