Re: Persist injection points across server restarts

On Mon, May 26, 2025 at 01:17:46PM +0530, Rahila Syed wrote:
> It appears that Github CI is reporting failures with
> injection_points/002_data_persist
> failing across all OSes.

I am not sure to see what you are referring to here, based on the
following reports:
https://commitfest.postgresql.org/patch/5731/
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F5731
http://cfbot.cputube.org/highlights/all.html

The last failure reported that I know of was due to the addition of
the runtime arguments to the macro INJECTION_POINT().

> I wonder if it would be better to include a check for the return value of
> `durable_rename` to
> manage potential failure scenarios.

Or we can be smarter and make this call an ERROR.

> 2. +
> +       file = AllocateFile(INJ_DUMP_FILE ".tmp", PG_BINARY_W);
> 
> Could we perhaps add a brief comment clarifying the rationale behind the
> use of a
> temporary file in this context?

Sure.  This is about avoiding the load of incomplete files if there's
a crash in the middle of a flush call, for example.

> It seems that `buf` should be null-terminated before being passed to
> `pstrdup(buf)`.
> Otherwise, `strlen` might not accurately determine the length. This seems
> to be
> consistent with the handling of `fread` calls in `snapmgr.c`.

Hmm, good point.  That would be safer, even if the write part should
already push the null termination.
--
Michael