On 2023-Nov-17, Dilip Kumar wrote: > On Thu, Nov 16, 2023 at 3:11 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> > wrote: > > > > I just noticed that 0003 does some changes to > > TransactionGroupUpdateXidStatus() that haven't been adequately > > explained AFAICS. How do you know that these changes are safe? > > IMHO this is safe as well as logical to do w.r.t. performance. It's > safe because whenever we are updating any page in a group we are > acquiring the respective bank lock in exclusive mode and in extreme > cases if there are pages from different banks then we do switch the > lock as well before updating the pages from different groups.
Looking at the coverage for this code, https://coverage.postgresql.org/src/backend/access/transam/clog.c.gcov.html#413 it seems in our test suites we never hit the case where there is anything in the "nextidx" field for commit groups. To be honest, I don't understand this group stuff, and so I'm doubly hesitant to go without any testing here. Maybe it'd be possible to use Michael Paquier's injection points somehow? I think in the code comments where you use "w.r.t.", that acronym can be replaced with "for", which improves readability. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "All rings of power are equal, But some rings of power are more equal than others." (George Orwell's The Lord of the Rings)
