Re: [Patch] Make pg_checksums skip foreign tablespace directories

Thank you David for decrypting my previous mail.., and your
translation was correct.

At Fri, 21 Feb 2020 15:07:12 +0900, Michael Paquier <michael(at)paquier(dot)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