From: | Arseniy Mukhin <arseniy(dot)mukhin(dot)dev(at)gmail(dot)com> |
---|---|
To: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Rishu Bagga <rishu(dot)postgres(at)gmail(dot)com>, Yura Sokolov <y(dot)sokolov(at)postgrespro(dot)ru>, Daniil Davydov <3danissimo(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Alexandra Wang <alexandra(dot)wang(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Joel Jacobson <joel(at)compiler(dot)org> |
Subject: | Re: LISTEN/NOTIFY bug: VACUUM sets frozenxid past a xid in async queue |
Date: | 2025-09-25 09:43:34 |
Message-ID: | CAE7r3MLe8XTALJeBK+2BfDsB0n1KkZqZ1kdaCqEhf8--xcf9JA@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Sep 24, 2025 at 10:23 PM Matheus Alcantara
<matheusssilv97(at)gmail(dot)com> wrote:
>
> On Wed Sep 24, 2025 at 10:59 AM -03, Arseniy Mukhin wrote:
> > ...
> > Patch looks great. Some minor points:
> >
> > I have a warning when using git am with the patch:
> > warning: 1 line adds whitespace errors.
> >
> I don't think that this is a problem? And I don't know how to remove
> this warning, I've just created the patch with git format-patch @~1
>
In my experience a run of pgindent usually fixes all such warnings,
but I agree it's not a big deal.
>
> > What do you think about a Assert in asyncQueueRollbackNotifications()
> > that other backends still see us as 'in progress'? So we can be sure
> > that they can't process our notifications before we mark notifications
> > as 'committed=false'. Not sure how to do it correctly, maybe
> >
> > Assert(TransactionIdIsValid(MyProc->xid));
> >
> > will work? The TAP test that I tried to write also should test it.
> >
> Sounds resanable to me. I think that we could use the following assert
> when iterating over the entries, what do you think?
> Assert(TransactionIdIsInProgress(qe->xid));
>
Yeah, TransactionIdIsInProgress() looks like the right way to do it.
And one more 'Assert' idea... maybe we can add
Assert(TransactionIdIsCurrentTransactionId(qe->xid)) just to verify
that we are only touching our own notifications?
Thank you, the fixes look good.
Best regards,
Arseniy Mukhin
From | Date | Subject | |
---|---|---|---|
Next Message | Álvaro Herrera | 2025-09-25 09:46:28 | Re: NOT NULL NOT ENFORCED |
Previous Message | Alexander Kukushkin | 2025-09-25 09:31:11 | Re: tiny step toward threading: reduce dependence on setlocale() |