Re: [Patch] ALTER SYSTEM READ ONLY

Dilip Kumar <dilipbalaut@gmail.com>

From: Dilip Kumar <dilipbalaut@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amul Sul <sulamul@gmail.com>, Andres Freund <andres@anarazel.de>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-03-08T11:06:27Z
Lists: pgsql-hackers
On Wed, Mar 3, 2021 at 8:56 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Mar 2, 2021 at 7:22 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > Why do we need to move promote related code in XLogAcceptWrites?
> > IMHO, this promote related handling should be in StartupXLOG only.
> > That will look cleaner.
>
> The key design question here, at least in my mind, is what exactly
> happens after prohibit-WAL + system-crash + recovery-finishes. We
> clearly can't write the checkpoint or end-of-recovery record and
> proceed with business as usual, but are we still in recovery? Either
> (1) we are technically still in recovery, stopping just short of
> entering normal running, and will emerge from recovery when WAL is
> permitted again; or (2) we have technically finished recovery, but
> deferred some of the actions that would normally occur at that time
> until a later point. Maybe this is academic distinction as much as
> anything, but the idea is if we choose #1 then we should do as little
> as possible at the point when recovery finishes and defer as much as
> possible until we actually enter normal running; whereas if we choose
> #2 we should do as much as possible at the point when recovery
> finishes and defer only those things which absolutely have to be
> deferred. That said, I and I think also Andres are voting for #2.
>
> But if we go that way, that precludes what you are proposing here. If
> we picked #1 then it would be natural for the startup process to
> remain active and the control file update to be postponed until WAL
> writes are re-enabled; but under model #2 we want, if possible, for
> the startup process to exit and the control file update to happen
> normally, and only the writing of the actual WAL records to be
> deferred.

Maybe I did not put my point clearly,  let me clarify that.  First, I
was also inclined that it should work like #2.  And, if it works like
#2 then I would assume that the code goes in XLogAcceptWrites function
should be minimal, only those part which we want to execute after the
system is back to read-write mode.  So basically, the XLogAcceptWrites
should only keep the code that is common code which we want to execute
at the end of the StartupXLog if the system is normal or we want to
execute when the system is back to read-write if it was read only.  So
my point was all the uncommon code that we have moved into
XLogAcceptWrites should be kept inside the StartupXLog function only.
So I think the promotion-related code doesn't belong to the
XLogAcceptWrites function.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com



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.