Re: Optimize LISTEN/NOTIFY
| От | Joel Jacobson |
|---|---|
| Тема | Re: Optimize LISTEN/NOTIFY |
| Дата | |
| Msg-id | 26d4dd6a-cfc5-4efc-9704-9cd3216ed712@app.fastmail.com обсуждение исходный текст |
| Ответ на | Re: Optimize LISTEN/NOTIFY ("Joel Jacobson" <joel@compiler.org>) |
| Список | pgsql-hackers |
On Sat, Nov 8, 2025, at 13:59, Joel Jacobson wrote: > On Fri, Nov 7, 2025, at 19:59, Joel Jacobson wrote: >> * The logic in SignalBackends has been reworked and simplified, >> thanks to the new isAdvancing and advancingPos fields. >> I now think it's finally easy to reason about why each branch >> in SignalBackends must be correct. > > I was wrong. I wrongly assumed asyncQueueReadAllNotifications would read > up until head, which it might not actually do: > > * Process messages up to the stop position, end of page, or an > * uncommitted message. > > This in turn could cause a listening backend to remain behind, if there > would be no more notifies, so it unfortunately seems like we will always > need to signal when a backend isAdvancing, and therefore have no use of > the advancingPos field. > > I will do more correctness and benchmark testing before posting a new > version. Just wanted to give you a heads up on the bug, so you don't > waste time reviewing. Changes since v24: * Removed the QueuePosition advancingPos QueueBackendStatus field. * Always signal when isAdvancing is true. Some benchmarking: master: % ./pg_async_notify_test --listeners 1 --notifiers 1 --channels 1000 --sleep 0.1 --sleep-exp 1.01 13 s: 8668 sent (658/s), 8676 received (653/s) Notification Latency Distribution: 0.00-0.01ms 0 (0.0%) avg: 0.000ms 0.01-0.10ms 0 (0.0%) avg: 0.000ms 0.10-1.00ms # 3 (0.0%) avg: 0.783ms 1.00-10.00ms # 49 (0.6%) avg: 5.559ms 10.00-100.00ms # 168 (1.9%) avg: 56.360mss >100.00ms ######### 8456 (97.5%) avg: 256.086ms v25: % ./pg_async_notify_test --listeners 1 --notifiers 1 --channels 1000 --sleep 0.1 --sleep-exp 1.01 14 s: 27097 sent (1959/s), 27097 received (1960/s) Notification Latency Distribution: 0.00-0.01ms 0 (0.0%) avg: 0.000ms 0.01-0.10ms # 962 (3.6%) avg: 0.081ms 0.10-1.00ms ######### 25066 (92.5%) avg: 0.321ms 1.00-10.00ms # 1069 (3.9%) avg: 2.104ms 10.00-100.00ms 0 (0.0%) avg: 0.000ms >100.00ms 0 (0.0%) avg: 0.000ms On master, I see lots of "waiting for AccessExclusiveLock on object 0" in the logs for this benchmark setup, but none at all for v25. I wonder if there would be some way to solve the problem with v24, without having to always signal when isAdvancing is true with an advancingPos at or ahead of the notifier's queueHeadAfterWrite. One idea I had, that I think is a bad idea, but just mentioning it in case you think it could work, is to check if the new pos is at head, in asyncQueueReadAllNotifications's PG_FINALLY block, and if not, to let the listening backend signal itself. I guess this is a bad idea, since then it could signal itself over and over again, and consume a lot of resources, if some uncommitted message takes a long time to commit. The current mechanism limit the rate of wake-ups to the same rate as the notifies, which seems sensible. Thoughts on this? /Joel
Вложения
В списке pgsql-hackers по дате отправления: