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

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Thomas Munro <thomas.munro@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, "Bossart, Nathan" <bossartn@amazon.com>, Tom Lane <tgl@sss.pgh.pa.us>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-07-11T20:25:33Z
Lists: pgsql-hackers
Hi,

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?

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

Greetings,

Andres Freund



Commits

  1. Expand the use of get_dirent_type(), shaving a few calls to stat()/lstat()

  2. fsync pg_logical/mappings in CheckPointLogicalRewriteHeap().

  3. Introduce logical decoding.