Re: Adding facility for injection points (or probe points?) for more advanced tests

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Adding facility for injection points (or probe points?) for more advanced tests
Date: 2024-01-12 04:10:38
Message-ID: CAExHW5sc_ar7=W9XCcC9TwYxZF71Ghc6poQ_+u4HXTXmNB7KAw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 12, 2024 at 5:05 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Thu, Jan 11, 2024 at 02:47:27PM +0530, Ashutosh Bapat wrote:
> > On Thu, Jan 11, 2024 at 9:42 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >> I don't really aim for complicated here, just useful.
> >
> > It isn't complicated. Such simple error check improve user's
> > confidence on the feature and better be part of the 1st cut.
>
> I'm really not sure about that, because it does not impact the scope
> of the facility even with all the use cases I've seen where injection
> points could be used. It could always be added later if there's a
> strong push for it. For testing, I'm biased about attempting to load
> callbacks in the process attaching them.
>

I am not able to understand the objection to adding another handful of
lines of code. The core code is quite minimal and better to be robust.
We may seek someone else's opinion to break the tie.

> >>> With v6 I could run the test when built with enable_injection_point
> >>> false. I just ran make check in that folder. Didn't test meson build.
> >>
> >> The CI has been failing because 041_invalid_checkpoint_after_promote
> >> was loading Time::HiRes::nanosleep and Windows does not support it.
> >
> > Some miscommunication here. The SQL test under injection_point module
> > can be run in a build without injection_point and it fails. I think
> > it's better to have an alternate output for the same or prohibit the
> > test running itself.
>
> The same problem exists if you try to run the SSL tests in
> src/test/ssl/ without support build for them. Protections at the
> upper levels are good enough for the CI and the buildfarm, while
> making the overall maintenance cheaper, so I'm happy with just these.
>
> It also seems like you've missed this message, where this has been
> mentioned (spoiler: first version of the patch used an alternate
> output):
> https://www.postgresql.org/message-id/ZUnuzPimkZCOVEcz@paquier.xyz

Ah! sorry for missing that. If there's a precedent, I am ok. If the
confusion arises we can fix it later.

--
Best Wishes,
Ashutosh Bapat

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2024-01-12 04:14:30 reorganize "Shared Memory and LWLocks" section of docs
Previous Message Amit Kapila 2024-01-12 03:57:54 Re: A failure in t/038_save_logical_slots_shutdown.pl