On Wed, Mar 06, 2024 at 10:19:41AM +0100, Jelte Fennema-Nio wrote:
> What I mainly meant is that anything in src/test/modules is not even
> close to somewhat useful for other people to use. They are really just
> very specific tests that need to be written in C. Afaict all those
> modules are not
On Wed, 6 Mar 2024 at 07:17, Michael Paquier wrote:
> I'm open to that if there's enough demand for it, but I
> don't know how much we should accomodate with the existing
> requirements of contrib/ for something that's only developer-oriented.
There's quite a few developer-oriented GUCs as well,
On Tue, Mar 05, 2024 at 09:43:03AM +0100, Jelte Fennema-Nio wrote:
> I do think there is quite a bit of a difference from a user
> perspective to providing a few custom configure flags and having to go
> to a separate directory and run "make install" there too. Simply
> because it's yet another
On Mon, 4 Mar 2024 at 23:23, Michael Paquier wrote:
> In my experience, anybody who does serious testing with their product
> integrated with Postgres have two or three types of builds with their
> own scripts: one with assertions, -DG and other developer-oriented
> options enabled, and one for
On Mon, Mar 04, 2024 at 10:51:41AM +0100, Jelte Fennema-Nio wrote:
> Yeah, it makes sense that you'd want to backport fixes/changes to
> this. As long as you put a disclaimer in the docs that you can do that
> for this module, I think it would be fine. Our tests fairly regularly
> break anyway
On Mon, 4 Mar 2024 at 06:27, Michael Paquier wrote:
> I have mentioned that on a separate thread,
Yeah, I didn't read all emails related to this feature
> Perhaps we could consider that as an exception in "contrib", or have a
> separate path for test modules we're OK to install (the calls had
>
Hi,
On Mon, Mar 04, 2024 at 10:44:34AM +0900, Michael Paquier wrote:
> On Fri, Mar 01, 2024 at 06:52:45AM +, Bertrand Drouvot wrote:
> > + if (defined($backend_type))
> > + {
> > + $backend_type = qq('$backend_type');
> > + $die_message = "the backend type
On Mon, Mar 04, 2024 at 05:17:52AM +0100, Jelte Fennema-Nio wrote:
> I noticed this was committed, and took a quick look. It sounds very
> useful for testing Citus to be able to use injection points too, but I
> noticed this code is included in src/test/modules. It sounds like that
> location will
> On 4 Mar 2024, at 06:44, Michael Paquier wrote:
> so I have applied it
Great! Thank you! A really useful stuff for an asynchronous testing!
> On 4 Mar 2024, at 09:17, Jelte Fennema-Nio wrote:
>
> this code is included in src/test/modules. It sounds like that
> location will make it
I noticed this was committed, and took a quick look. It sounds very
useful for testing Citus to be able to use injection points too, but I
noticed this code is included in src/test/modules. It sounds like that
location will make it somewhat hard to install. If that's indeed the
case, would it be
On Fri, Mar 01, 2024 at 06:52:45AM +, Bertrand Drouvot wrote:
> + if (defined($backend_type))
> + {
> + $backend_type = qq('$backend_type');
> + $die_message = "the backend type $backend_type";
> + }
> + else
> + {
> + $backend_type =
On Mon, Feb 26, 2024 at 08:24:04AM +, Bertrand Drouvot wrote:
> I'll try to submit the POC patch in [1] before beginning of next week now that
> we're "just waiting" if there is more comments on this current thread.
I'll look at what you have there in more details.
> [1]:
>
Hi,
On Fri, Mar 01, 2024 at 11:02:01AM +0900, Michael Paquier wrote:
> On Thu, Feb 29, 2024 at 06:19:58PM +0500, Andrey M. Borodin wrote:
> > Works fine for me. Thanks!
>
> + "timed out waiting for the backend type to wait for the injection
> point name";
>
> The log should provide
On Thu, Feb 29, 2024 at 06:19:58PM +0500, Andrey M. Borodin wrote:
> Works fine for me. Thanks!
+ "timed out waiting for the backend type to wait for the injection
point name";
The log should provide some context about the caller failing, meaning
that the backend type and the injection
> On 29 Feb 2024, at 15:20, Bertrand Drouvot
> wrote:
>
> done in v2 attached.
Works fine for me. Thanks!
Best regards, Andrey Borodin.
Hi,
On Thu, Feb 29, 2024 at 02:34:35PM +0500, Andrey M. Borodin wrote:
>
>
> > On 29 Feb 2024, at 13:35, Bertrand Drouvot
> > wrote:
> >
> > Something like the attached?
>
> There's extraneous print "done\n";
doh! bad copy/paste ;-)
> Also can we have optional backend type :)
done in v2
> On 29 Feb 2024, at 13:35, Bertrand Drouvot
> wrote:
>
> Something like the attached?
There's extraneous print "done\n";
Also can we have optional backend type :)
Best regards, Andrey Borodin.
Hi,
On Thu, Feb 29, 2024 at 02:56:23PM +0900, Michael Paquier wrote:
> On Wed, Feb 28, 2024 at 06:20:41AM +, Bertrand Drouvot wrote:
> > On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
> >> On Tue, Feb 27, 2024 at 01:39:59PM +, Bertrand Drouvot wrote:
> >> > So, I'm ok
On Wed, Feb 28, 2024 at 06:20:41AM +, Bertrand Drouvot wrote:
> On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
>> On Tue, Feb 27, 2024 at 01:39:59PM +, Bertrand Drouvot wrote:
>> > So, I'm ok with the new helper too.
>>
>> If both of you feel strongly about that, I'm OK
On Wed, Feb 28, 2024 at 10:27:52AM +0500, Andrey M. Borodin wrote:
> BTW, if we had an SQL function such as injection_point_await(name),
> we could use it in our isolation tests as well as our TAP tests :)
I am not quite sure to follow here. The isolation test facility now
relies on the in-core
Hi,
On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
> On Tue, Feb 27, 2024 at 01:39:59PM +, Bertrand Drouvot wrote:
> > So, I'm ok with the new helper too.
>
> If both of you feel strongly about that, I'm OK with introducing
> something like that.
Thanks!
> Now, a routine
> On 28 Feb 2024, at 09:26, Michael Paquier wrote:
>
> Now, a routine should be only about waiting on
> pg_stat_activity to report something
BTW, if we had an SQL function such as injection_point_await(name), we could
use it in our isolation tests as well as our TAP tests :)
However, this
On Tue, Feb 27, 2024 at 01:39:59PM +, Bertrand Drouvot wrote:
> So, I'm ok with the new helper too.
If both of you feel strongly about that, I'm OK with introducing
something like that. Now, a routine should be only about waiting on
pg_stat_activity to report something, as for the logs we
Hi,
On Tue, Feb 27, 2024 at 04:49:03PM +0500, Andrey M. Borodin wrote:
> Hi,
>
> > On 27 Feb 2024, at 16:08, Bertrand Drouvot
> > wrote:
> >
> > On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
> >>
> >> But, AFAICS, the purpose is the same: wait until event happened.
> >
Hi,
> On 27 Feb 2024, at 16:08, Bertrand Drouvot
> wrote:
>
> On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
>>
>> But, AFAICS, the purpose is the same: wait until event happened.
>
> I think it's easier to understand the tests (I mean what the purpose of the
> injection
Hi,
On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
>
>
> > On 27 Feb 2024, at 04:29, Michael Paquier wrote:
> >
> > For
> > example, the test just posted here does not rely on that:
> >
> On 27 Feb 2024, at 04:29, Michael Paquier wrote:
>
> For
> example, the test just posted here does not rely on that:
> https://www.postgresql.org/message-id/zdyzya4yrnapw...@ip-10-97-1-34.eu-west-3.compute.internal
Instead, that test is scanning logs
+ # Note:
On Mon, Feb 26, 2024 at 02:10:49PM +0500, Andrey M. Borodin wrote:
> So that we could do something like
>
> ok(node_standby->await_injection_point(“CreateRestartPoint”,”checkpointer"));
It would be more flexible with a full string to describe the test
rather than a process name in the second
> On 26 Feb 2024, at 08:57, Michael Paquier wrote:
>
>
Would it be possible to have a helper function to check this:
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer'
Hi,
On Mon, Feb 26, 2024 at 12:57:09PM +0900, Michael Paquier wrote:
> On Thu, Feb 22, 2024 at 08:00:24AM +, Bertrand Drouvot wrote:
> > +/* Maximum number of wait usable in injection points at once */
> >
> > s/Maximum number of wait/Maximum number of waits/ ?
>
> Thanks. I've edited a
On Thu, Feb 22, 2024 at 08:00:24AM +, Bertrand Drouvot wrote:
> +/* Maximum number of wait usable in injection points at once */
>
> s/Maximum number of wait/Maximum number of waits/ ?
Thanks. I've edited a few more places while scanning the whole.
>
> 2 ===
>
> +# Check the logs that
Hi,
On Thu, Feb 22, 2024 at 12:02:01PM +0900, Michael Paquier wrote:
> On Wed, Feb 21, 2024 at 11:50:21AM +, Bertrand Drouvot wrote:
> > A few comments:
> >
> > 1 ===
> > I think "up" is missing at several places in the patch where "wake" is used.
> > I could be wrong as non native english
On Wed, Feb 21, 2024 at 11:50:21AM +, Bertrand Drouvot wrote:
> I think the approach is fine and the hardcoded value is "large" enough (it
> would
> be surprising, at least to me, to write a test that would need more than 32
> waiters).
This could use less. I've never used more than 3 of
Hi,
On Wed, Feb 21, 2024 at 04:46:00PM +0900, Michael Paquier wrote:
> On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> > Well, both you and Andrey are asking for it now, so let's do it. The
> > implementation is simple:
> > - Store in InjectionPointSharedState an array of
Hi,
On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> On Tue, Feb 20, 2024 at 03:55:08PM +, Bertrand Drouvot wrote:
> > +PG_FUNCTION_INFO_V1(injection_points_wake);
> > +Datum
> > +injection_points_wake(PG_FUNCTION_ARGS)
> > +{
> >
> > I think that This function will wake up
On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> Well, both you and Andrey are asking for it now, so let's do it. The
> implementation is simple:
> - Store in InjectionPointSharedState an array of wait_counts and an
> array of names. There is only one condition variable.
> -
On Tue, Feb 20, 2024 at 05:32:38PM +0300, Andrey M. Borodin wrote:
> I will try to simplify test to 2-step, but it would be much easier
> to implement if injection points could be awaken independently.
I don't mind implementing something that wakes only a given point
name, that's just more state
On Tue, Feb 20, 2024 at 03:55:08PM +, Bertrand Drouvot wrote:
> Okay, makes sense to keep this as it is as a "template" in case more stuff is
> added.
>
> + /* Counter advancing when injection_points_wake() is called */
> + int wait_counts;
>
> In that case
Hi,
On Tue, Feb 20, 2024 at 08:28:28AM +0900, Michael Paquier wrote:
> On Mon, Feb 19, 2024 at 02:28:04PM +, Bertrand Drouvot wrote:
> > If slock_t protects "only" one counter, then what about using
> > pg_atomic_uint64
> > or pg_atomic_uint32 instead? And btw do we need wait_counts at all?
> On 20 Feb 2024, at 02:21, Michael Paquier wrote:
>
> On Mon, Feb 19, 2024 at 11:54:20AM +0300, Andrey M. Borodin wrote:
>> 1. injection_points_wake() will wake all of waiters. But it's not
>> suitable for complex tests. I think there must be a way to wake only
>> specific waiter by
On Mon, Feb 19, 2024 at 02:28:04PM +, Bertrand Drouvot wrote:
> +CREATE FUNCTION injection_points_wake()
>
> what about injection_points_wakeup() instead?
Sure.
> +/* Shared state information for injection points. */
> +typedef struct InjectionPointSharedState
> +{
> + /* protects
On Mon, Feb 19, 2024 at 11:54:20AM +0300, Andrey M. Borodin wrote:
> 1. injection_points_wake() will wake all of waiters. But it's not
> suitable for complex tests. I think there must be a way to wake only
> specific waiter by injection point name.
I don't disagree with that, but I don't have a
Hi,
On Mon, Feb 19, 2024 at 04:51:45PM +0900, Michael Paquier wrote:
> On Mon, Feb 19, 2024 at 03:01:40PM +0900, Michael Paquier wrote:
> > 0002 is a polished version of the TAP test that makes use of this
> > facility, providing coverage for the bug fixed by 7863ee4def65
> > (reverting this
> On 19 Feb 2024, at 09:01, Michael Paquier wrote:
>
> Thoughts and comments are welcome.
Hi Michael,
thanks for your work on injection points! I want to test a bunch of stuff using
this facility.
I have a wishlist of functionality that I'd like to see in injection points. I
hope you
On Mon, Feb 19, 2024 at 03:01:40PM +0900, Michael Paquier wrote:
> 0002 is a polished version of the TAP test that makes use of this
> facility, providing coverage for the bug fixed by 7863ee4def65
> (reverting this commit causes the test to fail), where a restart point
> runs across a promotion
Hi all,
(Ashutosh in CC as he was involved in the discussion last time.)
I have proposed on the original thread related to injection points to
have more stuff to be able to wait at an arbtrary point and wake at
will the process waiting so as it is possible to control the order of
actions taken in
46 matches
Mail list logo
