On Thu, Jan 7, 2021 at 1:02 AM Amul Sul <sula...@gmail.com> wrote: > As per this comment, it seems to be that we don't really need this LW lock. We > could have something else instead if we are afraid of having multiple > checkpoints at any given time which isn't possible, btw.
Yeah, I think this lock is useless. In fact, I think it's harmful, because it makes large sections of the checkpointer code, basically all of it really, run with interrupts held off for no reason. It's not impossible that removing the lock could reveal bugs elsewhere: suppose we have segments of code that actually aren't safe to interrupt, but because of this LWLock, it never happens. But, if that happens to be true, I think we should just view those cases as bugs to be fixed. One thing that blunts the impact of this quite a bit is that the checkpointer doesn't use rely much on CHECK_FOR_INTERRUPTS() in the first place. It has its own machinery: HandleCheckpointerInterrupts(). Possibly that's something we should be looking to refactor somehow as well. -- Robert Haas EDB: http://www.enterprisedb.com