Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-25 Thread Robert Haas
On Mon, Jan 18, 2021 at 5:18 PM Tom Lane wrote: > Yeah. Digging further, it looks like I oversimplified things above: > we once launched special background-worker-like processes for checkpoints, > and there could be more than one at a time. Thanks. I updated the commit message to mention some

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-18 Thread Michael Paquier
On Mon, Jan 18, 2021 at 02:36:48PM -0500, Robert Haas wrote: > Here's a patch to remove CheckpointLock completely. For > ProcessInterrupts() to do anything, one of the following things would > have to be true: > > [...] > > So I don't see any problem with just ripping this out entirely, but > I'd

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-18 Thread Tom Lane
Robert Haas writes: > On Mon, Jan 18, 2021 at 3:25 PM Tom Lane wrote: >> If memory serves, the reason for the lock was that the CHECKPOINT >> command used to run the checkpointing code directly in the calling >> backend, so we needed it to keep more than one process from doing >> that at once.

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-18 Thread Robert Haas
On Mon, Jan 18, 2021 at 3:25 PM Tom Lane wrote: > If memory serves, the reason for the lock was that the CHECKPOINT > command used to run the checkpointing code directly in the calling > backend, so we needed it to keep more than one process from doing > that at once. AFAICS, it's no longer

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-18 Thread Tom Lane
Robert Haas writes: > Here's a patch to remove CheckpointLock completely. ... > So I don't see any problem with just ripping this out entirely, but > I'd like to know if anybody else does. If memory serves, the reason for the lock was that the CHECKPOINT command used to run the checkpointing

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-18 Thread Robert Haas
On Thu, Jan 14, 2021 at 10:21 AM Robert Haas wrote: > 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

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-14 Thread Robert Haas
On Thu, Jan 7, 2021 at 1:02 AM Amul Sul 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.

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-07 Thread Bharath Rupireddy
On Thu, Jan 7, 2021 at 1:22 PM Amul Sul wrote: > I am not sure I understood your point completely. Do you mean there could be > multiple bootstrap or standalone backends that could exist at a time and can > perform checkpoint? Actually, my understanding of the standalone backend was wrong

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-06 Thread Amul Sul
On Thu, Jan 7, 2021 at 12:45 PM Bharath Rupireddy wrote: > > On Thu, Jan 7, 2021 at 11:32 AM Amul Sul wrote: > > Snip from CreateCheckPoint(): > > -- > > /* > > * Acquire CheckpointLock to ensure only one checkpoint happens at a time. > > * (This is just pro forma, since in the present system

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-06 Thread Dilip Kumar
On Thu, Jan 7, 2021 at 12:45 PM Bharath Rupireddy wrote: > > On Thu, Jan 7, 2021 at 11:32 AM Amul Sul wrote: > > Snip from CreateCheckPoint(): > > -- > > /* > > * Acquire CheckpointLock to ensure only one checkpoint happens at a time. > > * (This is just pro forma, since in the present system

Re: CheckpointLock needed in CreateCheckPoint()?

2021-01-06 Thread Bharath Rupireddy
On Thu, Jan 7, 2021 at 11:32 AM Amul Sul wrote: > Snip from CreateCheckPoint(): > -- > /* > * Acquire CheckpointLock to ensure only one checkpoint happens at a time. > * (This is just pro forma, since in the present system structure there is > * only one process that is allowed to issue

CheckpointLock needed in CreateCheckPoint()?

2021-01-06 Thread Amul Sul
Hi ALL, Snip from CreateCheckPoint(): -- /* * Acquire CheckpointLock to ensure only one checkpoint happens at a time. * (This is just pro forma, since in the present system structure there is * only one process that is allowed to issue checkpoints at any given * time.) */