Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
От | Matheus Alcantara |
---|---|
Тема | Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue |
Дата | |
Msg-id | CAFY6G8ckPN10qO8koa+9pWVyD=RSU4tL2HNYh6wmGvRjtRWWNg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue (Yura Sokolov <y.sokolov@postgrespro.ru>) |
Список | pgsql-hackers |
Thanks for the comments! On Tue Sep 2, 2025 at 4:37 AM -03, Yura Sokolov wrote: > 29.08.2025 01:31, Matheus Alcantara пишет: >> On Thu Aug 21, 2025 at 8:14 PM -03, Matheus Alcantara wrote: >>> I'll work on this considering the initial Daniil comments at [1] >>> >>> [1] https://www.postgresql.org/message-id/CAJDiXgg1ArRB1-6wLtXfVVnQ38P9Y%2BCDfEc9M2TXiOf_4kfBMg%40mail.gmail.com >>> >> I've been working on this on the last few days, please see the attached >> patch version. >> >> In this new version I tried to follow the suggestion from Daniil of >> scanning all pages from tail to head of the async queue. > > Since we still need to scan those pages, couldn't we mark finished > transactions as committed/aborted? > This way we may to not hold datfrozenxid for a long time and will allow > both safe clog truncation and safe async queue notification. > More over, most notifications could be marked as completed in > AtCommit_Notify already. So there is a need to recheck a few notifications > that were written but not marked in AtCommit_Notify. > > Since AsyncQueueEntry field is used only to check visibility, looks like it > is safe to set it to FrozenTransactionId. > Maybe we could have a boolean "committed" field on AsyncQueueEntry and check this field before sending the notification instead of call TransactionIdDidCommit()? Is something similar what you are proposing? We create the AsyncQueueEntry's and add into the SLRU at PreCommit_Notify(), so to mark these entries as committed on AtCommit_Notify() we would need a way to find these entries on the SLRU from the pendingNotifies->events that contains the notifications added on the current transaction being committed. This idea looks promising to me TBH, I'm just not sure if it's completely safe to mark an AsyncQueueEntry on the queue as committed without checking on TransactionIdDidCommit(). I would like to read more opinions about this before working on the next version based on this idea. -- Matheus Alcantara
В списке pgsql-hackers по дате отправления: