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
