Hi, On Mon, Feb 19, 2024 at 03:14:07PM +0900, Michael Paquier wrote: > On Thu, Feb 15, 2024 at 11:24:59AM +0000, Bertrand Drouvot wrote: > > Thanks for looking at it! > > > > Agree, using an assertion instead in v3 attached. > > And you did not forget the PG_USED_FOR_ASSERTS_ONLY.
Thanks to the "CompilerWarnings" cirrus test ;-) > > > > It seems important to document why we are saving this data here; while > > > we hold the LWLock for repslots, before we perform any termination, > > > and we want to protect conflict reports with incorrect data if the > > > slot data got changed while the lwlock is temporarily released while > > > there's a termination. > > > > Yeah, comments added in v3. > > The contents look rather OK, I may do some word-smithing for both. Thanks! > >> For example just after the kill()? It seems to me > >> that we should stuck the checkpointer, perform a forced upgrade of the > >> xmins, release the checkpointer and see the effects of the change in > >> the second loop. Now, modules/injection_points/ does not allow that, > >> yet, but I've posted a patch among these lines that can stop a process > >> and release it using a condition variable (should be 0006 on [1]). I > >> was planning to start a new thread with a patch posted for the next CF > >> to add this kind of facility with a regression test for an old bug, > >> the patch needs a few hours of love, first. I should be able to post > >> that next week. > > > > Great, that looks like a good idea! > > I've been finally able to spend some time on what I had in mind and > posted it here for the next CF: > https://www.postgresql.org/message-id/zdluxbk5hgpol...@paquier.xyz > > You should be able to reuse that the same way I do in 0002 posted on > the thread, where I'm causing the checkpointer to wait, then wake it > up. Thanks! I'll look at it. > > Are we going to fix this on master and 16 stable first and then later on > > add a > > test on master with the injection points? > > Still, the other patch is likely going to take a couple of weeks > before getting into the tree, so I have no objection to fix the bug > first and backpatch, then introduce a test. Things have proved that > failures could show up in the buildfarm in this area, so more time > running this code across two branches is not a bad concept, either. Fully agree. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
