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