Hi Michael,
On 2/24/20 7:26 AM, Michael Paquier wrote:
On Sun, Feb 23, 2020 at 04:08:58PM +0900, Michael Paquier wrote:
Good idea. Let's do things as you suggest.
Applied and back-patched this one down to 11.
FWIW, we took a slightly narrower approach to this issue in the
pgBackRest patch (attached).
I don't have an issue with the prefix approach since it works and the
Postgres project is very likely to catch it if there is a change in
behavior.
For third-party projects, though, it might pay to be more conservative
in case the behavior changes in the future, i.e.
pg_internal.init[something] (but not pg_internal\.init[0-9]+) becomes valid.
Regards,
--
-David
da...@pgmasters.net
diff --git a/doc/xml/release.xml b/doc/xml/release.xml
index a9a61bc0..0089d222 100644
--- a/doc/xml/release.xml
+++ b/doc/xml/release.xml
@@ -67,6 +67,16 @@
</release-feature-list>
<release-improvement-list>
+ <release-item>
+ <release-item-contributor-list>
+ <release-item-ideator id="michael.paquier"/>
+ <release-item-contributor id="david.steele"/>
+ <release-item-reviewer id="cynthia.shang"/>
+ </release-item-contributor-list>
+
+ <p>Skip <file>pg_internal.init</file> temp file during
backup.</p>
+ </release-item>
+
<release-item>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>
@@ -8039,6 +8049,11 @@
<contributor-id type="github">mibiio</contributor-id>
</contributor>
+ <contributor id="michael.paquier">
+ <contributor-name-display>Michael
Paquier</contributor-name-display>
+ <contributor-id type="github">michaelpq</contributor-id>
+ </contributor>
+
<contributor id="michael.renner">
<contributor-name-display>Michael Renner</contributor-name-display>
<contributor-id type="github">terrorobe</contributor-id>
diff --git a/src/info/manifest.c b/src/info/manifest.c
index a5662fd6..1eae49af 100644
--- a/src/info/manifest.c
+++ b/src/info/manifest.c
@@ -579,8 +579,13 @@ manifestBuildCallback(void *data, const StorageInfo *info)
strPtr(manifestName));
}
- // Skip pg_internal.init since it is recreated on startup
- if (strEqZ(info->name, PG_FILE_PGINTERNALINIT))
+ // Skip pg_internal.init since it is recreated on startup. It's
also possible, (though unlikely) that a temp file with
+ // the creating process id as the extension can exist so skip that
as well. This seems to be a bug in PostgreSQL since
+ // the temp file should be removed on startup. Use
regExpMatchOne() here instead of preparing a regexp in advance since
+ // the likelihood of needing the regexp should be very small.
+ if ((pgVersion <= PG_VERSION_84 || buildData.dbPath) &&
strBeginsWithZ(info->name, PG_FILE_PGINTERNALINIT) &&
+ (strSize(info->name) == sizeof(PG_FILE_PGINTERNALINIT) - 1 ||
+ regExpMatchOne(STRDEF("\\.[0-9]+"), strSub(info->name,
sizeof(PG_FILE_PGINTERNALINIT) - 1))))
{
FUNCTION_TEST_RETURN_VOID();
return;
diff --git a/test/src/module/info/manifestTest.c
b/test/src/module/info/manifestTest.c
index 684905d1..9d945152 100644
--- a/test/src/module/info/manifestTest.c
+++ b/test/src/module/info/manifestTest.c
@@ -252,6 +252,10 @@ testRun(void)
// global directory
storagePathCreateP(storagePgWrite, STRDEF(PG_PATH_GLOBAL), .mode =
0700, .noParentCreate = true);
storagePutP(storageNewWriteP(storagePgWrite, STRDEF(PG_PATH_GLOBAL "/"
PG_FILE_PGINTERNALINIT)), NULL);
+ storagePutP(storageNewWriteP(storagePgWrite, STRDEF(PG_PATH_GLOBAL "/"
PG_FILE_PGINTERNALINIT ".1")), NULL);
+ storagePutP(
+ storageNewWriteP(storagePgWrite, STRDEF(PG_PATH_GLOBAL "/"
PG_FILE_PGINTERNALINIT ".allow"), .modeFile = 0400,
+ .timeModified = 1565282114), NULL);
storagePutP(
storageNewWriteP(storagePgWrite, STRDEF(PG_PATH_GLOBAL "/t1_1"),
.modeFile = 0400, .timeModified = 1565282114), NULL);
@@ -282,6 +286,7 @@ testRun(void)
"\n"
"[target:file]\n"
"pg_data/PG_VERSION={\"size\":4,\"timestamp\":1565282100}\n"
+
"pg_data/global/pg_internal.init.allow={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/global/t1_1={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_dynshmem/BOGUS={\"size\":0,\"timestamp\":1565282101}\n"
"pg_data/pg_notify/BOGUS={\"size\":0,\"timestamp\":1565282102}\n"
@@ -419,6 +424,7 @@ testRun(void)
"pg_data/base/1/t1_1.1={\"size\":0,\"timestamp\":1565282113}\n"
"pg_data/base/1/t8888888_8888888_vm={\"size\":0,\"timestamp\":1565282113}\n"
"pg_data/base/1/t8888888_8888888_vm.999999={\"size\":0,\"timestamp\":1565282113}\n"
+
"pg_data/global/pg_internal.init.allow={\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/global/t1_1={\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_dynshmem/BOGUS={\"master\":true,\"size\":0,\"timestamp\":1565282101}\n"
"pg_data/pg_hba.conf={\"master\":true,\"size\":9,\"timestamp\":1565282117}\n"
@@ -537,6 +543,7 @@ testRun(void)
"pg_data/base/1/555_init.1={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_vm.1={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_vm.1_vm={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
+
"pg_data/global/pg_internal.init.allow={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_dynshmem/BOGUS={\"size\":0,\"timestamp\":1565282101}\n"
"pg_data/pg_hba.conf={\"size\":9,\"timestamp\":1565282117}\n"
"pg_data/pg_replslot/BOGUS={\"size\":0,\"timestamp\":1565282103}\n"
@@ -625,6 +632,7 @@ testRun(void)
"pg_data/base/1/555_init={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_init.1={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_vm.1_vm={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
+
"pg_data/global/pg_internal.init.allow={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_dynshmem/BOGUS={\"size\":0,\"timestamp\":1565282101}\n"
"pg_data/pg_hba.conf={\"size\":9,\"timestamp\":1565282117}\n"
"pg_data/pg_replslot/BOGUS={\"size\":0,\"timestamp\":1565282103}\n"
@@ -705,6 +713,7 @@ testRun(void)
"pg_data/base/1/555_init={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_init.1={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/base/1/555_vm.1_vm={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
+
"pg_data/global/pg_internal.init.allow={\"master\":false,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_dynshmem/BOGUS={\"size\":0,\"timestamp\":1565282101}\n"
"pg_data/pg_hba.conf={\"size\":9,\"timestamp\":1565282117}\n"
"pg_data/pg_replslot/BOGUS={\"size\":0,\"timestamp\":1565282103}\n"
@@ -852,6 +861,7 @@ testRun(void)
"pg_data/base/1/PG_VERSION={\"size\":0,\"timestamp\":1565282120}\n"
"pg_data/base/1/pg_filenode.map={\"size\":0,\"timestamp\":1565282120}\n"
"pg_data/global/pg_control={\"master\":true,\"size\":0,\"timestamp\":1565282101}\n"
+
"pg_data/global/pg_internal.init.allow={\"checksum-page\":true,\"size\":0,\"timestamp\":1565282114}\n"
"pg_data/pg_clog/BOGUS={\"size\":0,\"timestamp\":1565282121}\n"
"pg_data/pg_hba.conf={\"master\":true,\"size\":9,\"timestamp\":1565282117}\n"
"pg_data/pg_multixact/BOGUS={\"size\":0,\"timestamp\":1565282101}\n"
@@ -903,6 +913,7 @@ testRun(void)
storageRemoveP(storageTest, STRDEF("pg/pg_tblspc/2"), .errorOnMissing
= true);
storagePathRemoveP(storageTest, STRDEF("ts/2"), .recurse = true);
+ storageRemoveP(storagePgWrite, STRDEF(PG_PATH_GLOBAL "/"
PG_FILE_PGINTERNALINIT ".allow"), .errorOnMissing = true);
//
-------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("manifest with all features - 12, online");
