On Thu, Jan 7, 2021 at 1:02 AM Amul Sul <sulamul@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