Thank you David for decrypting my previous mail.., and your
translation was correct.
At Fri, 21 Feb 2020 15:07:12 +0900, Michael Paquier <mich...@paquier.xyz> wrote
in
> On Thu, Feb 20, 2020 at 07:37:15AM -0600, David Steele wrote:
> > But since the name includes the backend's pid you would need to get lucky
> > and have a new backend with the same pid create the file after a restart. I
> > tried it and the old temp file was left behind after restart and first
> > connection to the database.
> >
> > I doubt this is a big issue in the field, but it seems like it would be nice
> > to do something about it.
>
> The natural area to do that would be around ResetUnloggedRelations().
> Still that would require combine both operations to not do any
> unnecessary lookups at the data folder paths.
>
> > I'm not excited about the amount of code duplication between these three
> > tools. I know this was because of back-patching various issues in the past,
> > but I really think we need to unify these data structures/functions in HEAD.
>
> The lists are duplicated because we have never really figured out how
> to combine this code in one place. The idea was to have all the data
> folder path logic and the lists within one header shared between the
> frontend and backend but there was not much support for that on HEAD.
>
> >> For now, my proposal is to fix the prefix first, and then let's look
> >> at the business with tablespaces where needed.
> >
> > OK.
>
> I'll let this patch round for a couple of extra day, and revisit it at
> the beginning of next week.
Thank you for the version.
I didn't look it closer bat it looks in the direction I wanted.
At a quick look, the following section attracted my eyes.
+ if (strncmp(de->d_name,
excludeFiles[excludeIdx].name,
+
strlen(excludeFiles[excludeIdx].name)) == 0)
+ {
+ elog(DEBUG1, "file \"%s\" matching
prefix \"%s\" excluded from backup",
+ de->d_name,
excludeFiles[excludeIdx].name);
+ excludeFound = true;
+ break;
+ }
+ }
+ else
+ {
+ if (strcmp(de->d_name,
excludeFiles[excludeIdx].name) == 0)
+ {
+ elog(DEBUG1, "file \"%s\" excluded from
backup", de->d_name);
+ excludeFound = true;
+ break;
+ }
The two str[n]cmps are different only in matching length. I don't
think we don't need to differentiate the two message there, so we
could reduce the code as:
| cmplen = strlen(excludeFiles[].name);
| if (!prefix_patch)
| cmplen++;
| if (strncmp(d_name, excludeFilep.name, cmplen) == 0)
| ...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
