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 | DDN5L1ZDAQPK.IGXFQ6A8O0K3@gmail.com обсуждение исходный текст |
| Ответ на | Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue ("Joel Jacobson" <joel@compiler.org>) |
| Ответы |
Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue
|
| Список | pgsql-hackers |
On Sat Oct 18, 2025 at 2:43 AM -03, Joel Jacobson wrote: > On Fri, Oct 17, 2025, at 22:50, Arseniy Mukhin wrote: > What a funny coincidence that the approach in this patch, > has one similarity with the "Direct advancement" approach > in the patch in the "Optimize LISTEN/NOTIFY" [1] thread, > namely that we're both interested in QUEUE_HEAD before/after > we push the notifications into the queue, in PreCommit_Notify(). > > It looks to me like our new data structures are Interchangeable, > so I guess we probably want both patches to eventually settle > on one and the same? > > The differences I note between our queue head before/after code are: > > - In this patch, you are palloc'ing a struct with two fields. > In [1], we're using two separate static QueuePosition variables. > > - In this patch, you are taking/releasing a shared lock before/after > the loop to read QUEUE_HEAD and set previousHead/head. > In [1], we avoid the need of the shared lock, by doing the reads > within the existing exclusive lock inside the loop, but instead > therefore need a firstIteration bool, to know which is the first > iteration, and need to overwrite the after-var in each iteration. > > I don't think the noted differences above matter, both seems fine. > Yeah, I also think that both approach seems fine. I keep the v8 version with the palloc, if someone has any concern about this I'm open to switch to another approach. > Another thing I noticed in your patch that made me wonder, > is the naming of the new AsyncQueueEntry bool field, > which is given the name "committed". > > I think this name is not entirely faithful, since when set to true, > the entry has not been committed yet. > > How about negating the meaning of this boolean field? > To instead indicate when the entry has been rollbacked. > Then, it would clearly communicate just that. > > Maybe naming it something like "rollbacked" or "aborted"? > Good point. I've renamed this field on the attached v8 version. -- Matheus Alcantara
Вложения
В списке pgsql-hackers по дате отправления: