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 wait_counts and an > > array of names. There is only one condition variable. > > - When a point wants to wait, it takes the spinlock and looks within > > the array of names until it finds a free slot, adds its name into the > > array to reserve a wait counter at the same position, releases the > > spinlock. Then it loops on the condition variable for an update of > > the counter it has reserved. It is possible to make something more > > efficient, but at a small size it would not really matter. > > - The wakeup takes a point name in argument, acquires the spinlock, > > and checks if it can find the point into the array, pinpoints the > > location of the counter to update and updates it. Then it broadcasts > > the change. > > - The wait loop checks its counter, leaves its loop, cancels the > > sleep, takes the spinlock to unregister from the array, and leaves. > > > > I would just hardcode the number of points that can wait, say 5 of > > them tracked in shmem? Does that look like what you are looking at? > > I was looking at that, and it proves to work OK, so you can do stuff > like waits and wakeups for multiple processes in a controlled manner. > The attached patch authorizes up to 32 waiters. I have switched > things so as the information reported in pg_stat_activity is the name > of the injection point itself.
Thanks! 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). A few comments: 1 === +-- Wakes a condition variable I think "up" is missing at several places in the patch where "wake" is used. I could be wrong as non native english speaker though. 2 === + /* Counters advancing when injection_points_wakeup() is called */ + int wait_counts[INJ_MAX_WAIT]; uint32? (here and other places where counter is manipulated). 3 === + /* Remove us from the waiting list */ "Remove this injection wait name from the waiting list" instead? 4 === + * SQL function for waking a condition variable. s/a condition variable/an injection wait point/ ? 5 === +PG_FUNCTION_INFO_V1(injection_points_wakeup); +Datum +injection_points_wakeup(PG_FUNCTION_ARGS) Empty line missing before "Datum"? 6 === Also maybe some comments are missing above injection_point_init_state(), injection_init_shmem() but it's more a Nit. 7 === While at it, should we add a second injection wait point in t/041_invalid_checkpoint_after_promote.pl to check that they are wake up individually? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com