Обсуждение: pg_basebackup -F plain -R overwrites postgresql.auto.conf
Hi, I found that pg_basebackup -F plain -R *overwrites* postgresql.auto.conf taken from the primary server with new primary_conninfo setting, while pg_basebackup -F tar -R just *appends* it into the file. I think that this is a bug and pg_basebackup -F plain -R should *append* the setting. Thought? I attached the patch to fix the bug. This patch should be back-patch to v12. Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters
Вложения
Hello
Seems bug was introduced in caba97a9d9f4d4fa2531985fd12d3cd823da06f3 - in HEAD only
In REL_12_STABLE we have:
bool is_recovery_guc_supported = true;
if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_RECOVERY_GUC)
is_recovery_guc_supported = false;
snprintf(filename, MAXPGPATH, "%s/%s", basedir,
is_recovery_guc_supported ? "postgresql.auto.conf" : "recovery.conf");
cf = fopen(filename, is_recovery_guc_supported ? "a" : "w");
It looks correct: append mode for postgresql.auto.conf
In HEAD version is_recovery_guc_supported variable was replaced to inversed use_recovery_conf without change fopen
mode.
regards, Sergei
On 2020/02/10 17:23, Sergei Kornilov wrote: > Hello > > Seems bug was introduced in caba97a9d9f4d4fa2531985fd12d3cd823da06f3 - in HEAD only > > In REL_12_STABLE we have: > > bool is_recovery_guc_supported = true; > > if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_RECOVERY_GUC) > is_recovery_guc_supported = false; > > snprintf(filename, MAXPGPATH, "%s/%s", basedir, > is_recovery_guc_supported ? "postgresql.auto.conf" : "recovery.conf"); > > cf = fopen(filename, is_recovery_guc_supported ? "a" : "w"); > > It looks correct: append mode for postgresql.auto.conf > > In HEAD version is_recovery_guc_supported variable was replaced to inversed use_recovery_conf without change fopen mode. Yes! Thanks for pointing out that! So the patch needs to be applied only in master. Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters
On Mon, Feb 10, 2020 at 9:41 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > > > On 2020/02/10 17:23, Sergei Kornilov wrote: > > Hello > > > > Seems bug was introduced in caba97a9d9f4d4fa2531985fd12d3cd823da06f3 - in HEAD only > > > > In REL_12_STABLE we have: > > > > bool is_recovery_guc_supported = true; > > > > if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_RECOVERY_GUC) > > is_recovery_guc_supported = false; > > > > snprintf(filename, MAXPGPATH, "%s/%s", basedir, > > is_recovery_guc_supported ? "postgresql.auto.conf" : "recovery.conf"); > > > > cf = fopen(filename, is_recovery_guc_supported ? "a" : "w"); > > > > It looks correct: append mode for postgresql.auto.conf > > > > In HEAD version is_recovery_guc_supported variable was replaced to inversed use_recovery_conf without change fopen mode. > > Yes! Thanks for pointing out that! > So the patch needs to be applied only in master. +1. We should absolutely not be overwriting the auto conf. -- Magnus Hagander Me: https://www.hagander.net/ Work: https://www.redpill-linpro.com/
On 2020-Feb-10, Fujii Masao wrote: > > > On 2020/02/10 17:23, Sergei Kornilov wrote: > > Hello > > > > Seems bug was introduced in caba97a9d9f4d4fa2531985fd12d3cd823da06f3 - in HEAD only > > > > In REL_12_STABLE we have: > > > > bool is_recovery_guc_supported = true; > > > > if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_RECOVERY_GUC) > > is_recovery_guc_supported = false; > > > > snprintf(filename, MAXPGPATH, "%s/%s", basedir, > > is_recovery_guc_supported ? "postgresql.auto.conf" : "recovery.conf"); > > > > cf = fopen(filename, is_recovery_guc_supported ? "a" : "w"); > > > > It looks correct: append mode for postgresql.auto.conf > > > > In HEAD version is_recovery_guc_supported variable was replaced to inversed use_recovery_conf without change fopen mode. > > Yes! Thanks for pointing out that! > So the patch needs to be applied only in master. Yikes, thanks. Pushing in a minute. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Feb-10, Alvaro Herrera wrote: > On 2020-Feb-10, Fujii Masao wrote: > > Yes! Thanks for pointing out that! > > So the patch needs to be applied only in master. > > Yikes, thanks. Pushing in a minute. Actually, if you want to push it, be my guest. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020/02/11 0:28, Alvaro Herrera wrote: > On 2020-Feb-10, Alvaro Herrera wrote: > >> On 2020-Feb-10, Fujii Masao wrote: > >>> Yes! Thanks for pointing out that! >>> So the patch needs to be applied only in master. >> >> Yikes, thanks. Pushing in a minute. > > Actually, if you want to push it, be my guest. Yeah, I pushed the patch. Thanks! Regards, -- Fujii Masao NTT DATA CORPORATION Advanced Platform Technology Group Research and Development Headquarters