Horton

Thread

Commits

  1. Remove preprocessor guards from injection points

  2. Fix comments in recovery tests

  1. Remove obsolate comments from 047_checkpoint_physical_slot

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-09-25T01:39:43Z

    Dear hackers,
(CC: Alexander, who is an original committer)

While reviewing others, I found $SUBJECT.
Initially the tests inserted 2M tuples twice, and 4464fddf improves to use the advance_wal().
However, code comments in the test missed to be updated.
PSA the fix patch.

This exists PG17+, which has the same TAP test.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  2. RE: Remove obsolate comments from 047_checkpoint_physical_slot

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-09-25T07:23:33Z

    Dear hackers,

> While reviewing others, I found $SUBJECT.

I found another cleanup point related with this. In CreateCheckPoint():

```
#ifdef USE_INJECTION_POINTS
	INJECTION_POINT("checkpoint-before-old-wal-removal");
#endif
```

Here USE_INJECTION_POINTS check is not needed. If the feature is disabled,
the macro function would be ((void) name). IIUC, we are using the macro if if-branch
exists.

E.g., another injection point "create-restart-point" does not have the #ifdef part.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  3. Re: Remove obsolate comments from 047_checkpoint_physical_slot

    Daniel Gustafsson <daniel@yesql.se> — 2025-09-25T07:32:40Z

    > On 25 Sep 2025, at 09:23, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
> 
> Dear hackers,
> 
>> While reviewing others, I found $SUBJECT.
> 
> I found another cleanup point related with this. In CreateCheckPoint():
> 
> ```
> #ifdef USE_INJECTION_POINTS
> INJECTION_POINT("checkpoint-before-old-wal-removal");
> #endif
> ```
> 
> Here USE_INJECTION_POINTS check is not needed. If the feature is disabled,
> the macro function would be ((void) name). IIUC, we are using the macro if if-branch
> exists.

+1, that's not needed (and not used elsewhere in the code either).

--
Daniel Gustafsson

  4. Re: Remove obsolate comments from 047_checkpoint_physical_slot

    Michael Paquier <michael@paquier.xyz> — 2025-09-25T07:59:41Z

    On Thu, Sep 25, 2025 at 09:32:40AM +0200, Daniel Gustafsson wrote:
> On 25 Sep 2025, at 09:23, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:
>> Here USE_INJECTION_POINTS check is not needed. If the feature is disabled,
>> the macro function would be ((void) name). IIUC, we are using the macro if if-branch
>> exists.
> 
> +1, that's not needed (and not used elsewhere in the code either).

Yeah, let's remove that.  Simplification in the backend code is the
whole point of the double definition of the INJECTION_POINT() macro in
injection_point.h.

And the comments of the test 047 are indeed not required anymore.
Good catch.

If you want to go ahead and clean up all that, Daniel, please feel
free.  If not, I'm OK to pull the trigger on this one.
--
Michael

  5. Re: Remove obsolate comments from 047_checkpoint_physical_slot

    Daniel Gustafsson <daniel@yesql.se> — 2025-09-25T08:05:55Z

    > On 25 Sep 2025, at 09:59, Michael Paquier <michael@paquier.xyz> wrote:

> Yeah, let's remove that.  Simplification in the backend code is the
> whole point of the double definition of the INJECTION_POINT() macro in
> injection_point.h.

And I very much approve of that, it makes the code a lot better.

> If you want to go ahead and clean up all that, Daniel, please feel
> free.  If not, I'm OK to pull the trigger on this one.

No worries, I can fix it today.

--
Daniel Gustafsson

  6. RE: Remove obsolate comments from 047_checkpoint_physical_slot

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2025-09-25T11:11:17Z

    Dear Daniel, Michael,

Thanks for the reply and sorry for posting many times.
Both 046_checkpoint_logical and 047_checkpoint_physical has below comments:

```
# Run another checkpoint, this time in the background, and make it wait
# on the injection point) so that the checkpoint stops right before
# removing old WAL segments.
```

Is "injection point)" a typo? I feel it is enough to remove ")".
Feel free to include if you agree this point as well.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

  7. Re: Remove obsolate comments from 047_checkpoint_physical_slot

    Daniel Gustafsson <daniel@yesql.se> — 2025-09-25T18:31:44Z

    > On 25 Sep 2025, at 13:11, Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote:

> Is "injection point)" a typo? I feel it is enough to remove ")".
> Feel free to include if you agree this point as well.

Nice catch, I included this with the other fixes and pushed them today.

--
Daniel Gustafsson