Re: Multiple hosts in connection string failed to failover in non-hot standby mode
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
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-08T15:21:34Z
Lists: pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes: > Now, I also see that using pgwin32_setenv() instead of > src/port/setenv.c causes cl to be confused once we update > ecpg_regression.proj because it cannot find setenv(). Bringing the > question, why is it necessary to have both setenv.c and > pgwin32_setenv() on HEAD? setenv.c should be enough once you have the > fallback implementation of putenv() available. 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. It'd be okay to do it like that for the ECPG tests, perhaps, because we don't really care about small leaks in those. But I don't want it to happen across-the-board. Thinking more, the real problem is that use of libpgport goes hand-in-hand with #including port.h; it's not going to work real well if you do one without the other. And I don't think we want to include port.h in the ECPG test programs, because those are trying to model the environment that typical user applications see. Alternatives seem to be (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. (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). (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.) regards, tom lane
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