MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Fri, 16 Jun 2023 11:14:05 +0900
From: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Support to define custom wait events for extensions
In-Reply-To: <ZIrFEo5qLcNAGjTs@paquier.xyz>
References: <b9f5411acda0cf15c8fbb767702ff43e@oss.nttdata.com>
 <ZIrFEo5qLcNAGjTs@paquier.xyz>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <f3ce6711268cd443e71120eea3ba5cc0@oss.nttdata.com>
Archived-At: 
 <https://www.postgresql.org/message-id/f3ce6711268cd443e71120eea3ba5cc0%40oss.nttdata.com>
Precedence: bulk

Thanks for replying and your kind advice!

On 2023-06-15 17:00, Michael Paquier wrote:
> On Thu, Jun 15, 2023 at 03:06:01PM +0900, Masahiro Ikeda wrote:
>> In the core, the requested wait events are dynamically registered in 
>> shared
>> memory. The extension then obtains the wait event information with
>> GetNamedExtensionWaitEventTranche() and uses the value to notify the 
>> core
>> that it is waiting.
>> 
>> When a string representing of the wait event is requested,
>> it returns the name defined by calling
>> RequestNamedExtensionWaitEventTranche().
> 
> So this implements the equivalent of RequestNamedLWLockTranche()
> followed by GetNamedLWLockTranche() to get the wait event number,
> which can be used only during postmaster startup.  Do you think that
> we could focus on implementing something more flexible instead, that
> can be used dynamically as well as statically?  That would be similar
> to LWLockNewTrancheId() and LWLockRegisterTranche(), actually, where
> we would get one or more tranche IDs, then do initialization actions
> in shmem based on the tranche ID(s).

OK, I agree. I'll make a patch to only support
ExtensionWaitEventNewTrancheId() and ExtensionWaitEventRegisterTranche()
similar to LWLockNewTrancheId() and LWLockRegisterTranche().

>> 4. check the custom wait event
>> You can see the following results of psql-1.
>> 
>>             query            | wait_event_type |    wait_event
>> -----------------------------+-----------------+-------------------
>>  SELECT inject_wait_event(); | Extension       | custom_wait_event     
>>    #
>> requested wait event by the extension!
>> (1 row)
>> 
>> (..snip..)
> 
> A problem with this approach is that it is expensive as a test.  Do we
> really need one?  There are three places that set PG_WAIT_EXTENSION in
> src/test/modules/, more in /contrib, and there are modules like
> pg_stat_statements that could gain from events for I/O operations, for
> example.

Yes. Since it's hard to test, I thought the PoC extension
should not be committed. But, I couldn't figure out the best
way to test yet.

>> # TODOs
>> 
>> * tests on windows (since I tested on Ubuntu 20.04 only)
>> * add custom wait events for existing contrib modules (ex. 
>> postgres_fdw)
>> * add regression code (but, it seems to be difficult)
>> * others? (Please let me know)
> 
> Hmm.  You would need to maintain a state in a rather stable manner,
> and SQL queries can make that difficult in the TAP tests as the wait
> event information is reset each time a query finishes.  One area where
> I think this gets easier is with a background worker loaded with
> shared_preload_libraries that has a configurable naptime.  Looking at
> what's available in the tree, the TAP tests of pg_prewarm could use
> one test on pg_stat_activity with a custom wait event name
> (pg_prewarm.autoprewarm_interval is 0 hence the bgworker waits
> infinitely).  Note that in this case, you would need to be careful of
> the case where the wait event is loaded dynamically, but like LWLocks
> this should be able to work as well?

Thanks for your advice!

I tried to query on pg_stat_activity to check the background worker
invoked by pg_prewarm. But, I found that pg_stat_activity doesn't show
it although I may be missing something...

So, I tried to implement TAP tests. But I have a problem with it.
I couldn't find the way to check the status of another backend
while the another backend wait with custom wait events.

```
# TAP test I've implemented.

# wait forever with custom wait events in session1
$session1->query_safe("SELECT test_custom_wait_events_wait()");

# I want to check the wait event from another backend process
# But, the following code is never reached because the above
# query is waiting forever.
$session2->poll_query_until('postgres',
	qq[SELECT
		(SELECT count(*) FROM pg_stat_activity
	 		WHERE query ~ '^SELECT test_custom_wait_events_wait'
			  AND wait_event_type = 'Extension'
			  AND wait_event = 'custom_wait_event'
		) > 0;]);
```

If I'm missing something or you have any idea,
please let me know.

Now, I plan to

* find out more the existing tests to check wait events and locks
   (though I have already checked a little, but I couldn't find it)
* find another way to check wait event of the background worker invoked 
by extension
* look up the reason why pg_stat_activity doesn't show the background 
worker
* find a way to implement async queries in TAP tests

Regards,
-- 
Masahiro Ikeda
NTT DATA CORPORATION