Horton

Re: [Patch] ALTER SYSTEM READ ONLY

Soumyadeep Chakraborty <soumyadeep2007@gmail.com>

View thread View on postgr.es
From: Soumyadeep Chakraborty <soumyadeep2007@gmail.com>
To: Amul Sul <sulamul@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2020-07-24T17:10:19Z
Lists: pgsql-hackers 
On Thu, Jul 23, 2020 at 10:14 PM Amul Sul <sulamul@gmail.com> wrote:
>
> On Fri, Jul 24, 2020 at 6:28 AM Soumyadeep Chakraborty <soumyadeep2007@gmail.com> wrote:
> > In case it is necessary, the patch set does not wait for the checkpoint to
> > complete before marking the system as read-write. Refer:
> >
> > /* Set final state by clearing in-progress flag bit */
> > if (SetWALProhibitState(wal_state &
> ~(WALPROHIBIT_TRANSITION_IN_PROGRESS)))
> > {
> >   if ((wal_state & WALPROHIBIT_STATE_READ_ONLY) != 0)
> >     ereport(LOG, (errmsg("system is now read only")));
> >   else
> >   {
> >     /* Request checkpoint */
> >     RequestCheckpoint(CHECKPOINT_IMMEDIATE);
> >     ereport(LOG, (errmsg("system is now read write")));
> >   }
> > }
> >
> > We should RequestCheckpoint(CHECKPOINT_IMMEDIATE | CHECKPOINT_WAIT) before
> > we SetWALProhibitState() and do the ereport(), if we have a read-write
> > state change request.
> >
> +1, I too have the same question.
>
>
>
> FWIW, I don't we can request CHECKPOINT_WAIT for this place, otherwise, it
> think
> it will be deadlock case -- checkpointer process waiting for itself.

We should really just call CreateCheckPoint() here instead of
RequestCheckpoint().

> > 3. Some of the functions that were added such as GetWALProhibitState(),
> > IsWALProhibited() etc could be declared static inline.
> >
> IsWALProhibited() can be static but not GetWALProhibitState() since it
> needed to
> be accessible from other files.

If you place a static inline function in a header file, it will be
accessible from other files. E.g. pg_atomic_* functions.

Regards,
Soumyadeep

Commits

  1. Initialize variable to placate compiler.

  2. StartupXLOG: Don't repeatedly disable/enable local xlog insertion.

  3. StartupXLOG: Call CleanupAfterArchiveRecovery after XLogReportParameters.

  4. Postpone some end-of-recovery operations related to allowing WAL.

  5. Refactor some end-of-recovery code out of StartupXLOG().

  6. Re-enable contrib/bloom's TAP tests.

  7. Remove unnecessary call to ReadCheckpointRecord().

  8. Allow for error or refusal while absorbing a ProcSignalBarrier.

  9. Add comment to explain an unused function parameter

  10. Extend the ProcSignal mechanism to support barriers.

  11. At promotion, don't leave behind a partial segment on the old timeline.