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. > > 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. >> 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. > 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. -- Michael
signature.asc
Description: PGP signature