Re: Avoid erroring out when unable to remove or parse logical rewrite files to save checkpoint work

On Mon, Jul 11, 2022 at 01:25:33PM -0700, Andres Freund wrote:
> On 2022-04-08 13:18:57 -0700, Nathan Bossart wrote:
>> @@ -1035,32 +1036,9 @@ ParseConfigDirectory(const char *includedir,
>>  
>>          join_path_components(filename, directory, de->d_name);
>>          canonicalize_path(filename);
>> -        if (stat(filename, &st) == 0)
>> +        de_type = get_dirent_type(filename, de, true, elevel);
>> +        if (de_type == PGFILETYPE_ERROR)
>>          {
>> -            if (!S_ISDIR(st.st_mode))
>> -            {
>> -                /* Add file to array, increasing its size in blocks of 32 */
>> -                if (num_filenames >= size_filenames)
>> -                {
>> -                    size_filenames += 32;
>> -                    filenames = (char **) repalloc(filenames,
>> -                                            size_filenames * sizeof(char *));
>> -                }
>> -                filenames[num_filenames] = pstrdup(filename);
>> -                num_filenames++;
>> -            }
>> -        }
>> -        else
>> -        {
>> -            /*
>> -             * stat does not care about permissions, so the most likely reason
>> -             * a file can't be accessed now is if it was removed between the
>> -             * directory listing and now.
>> -             */
>> -            ereport(elevel,
>> -                    (errcode_for_file_access(),
>> -                     errmsg("could not stat file \"%s\": %m",
>> -                            filename)));
>>              record_config_file_error(psprintf("could not stat file \"%s\"",
>>                                                filename),
>>                                       calling_file, calling_lineno,
>> @@ -1068,6 +1046,18 @@ ParseConfigDirectory(const char *includedir,
>>              status = false;
>>              goto cleanup;
>>          }
>> +        else if (de_type != PGFILETYPE_DIR)
>> +        {
>> +            /* Add file to array, increasing its size in blocks of 32 */
>> +            if (num_filenames >= size_filenames)
>> +            {
>> +                size_filenames += 32;
>> +                filenames = (char **) repalloc(filenames,
>> +                                               size_filenames * sizeof(char *));
>> +            }
>> +            filenames[num_filenames] = pstrdup(filename);
>> +            num_filenames++;
>> +        }
>>      }
>>  
>>      if (num_filenames > 0)
> 
> Seems like the diff would be easier to read if it didn't move code around as
> much?

I opted to reorganize in order save an indent and simplify the conditions a
bit.  The alternative is something like this:

    if (de_type != PGFILETYPE_ERROR)
    {
        if (de_type != PGTFILETYPE_DIR)
        {
            ...
        }
    }
    else
    {
        ...
    }

I don't feel strongly one way or another, and I'll change it if you think
it's worth it to simplify the diff.

> Looks pretty reasonable, I'd be happy to commit it, I think.

Appreciate it.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com