Re: [Patch] ALTER SYSTEM READ ONLY

On Fri, Feb 19, 2021 at 5:43 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
>
> In the attached version I have made the changes accordingly what Robert has
> summarised in his previous mail[1].
>
> In addition to that, I also move the code that updates the control file to
> XLogAcceptWrites() which will also get skipped when the system is read-only (wal
> prohibited). The system will be in the crash recovery, and that will
> change once we do the end-of-recovery checkpoint and the WAL writes operation
> which we were skipping from startup. The benefit of keeping the system in
> recovery mode is that it fixes my concern[2] where other backends could connect
> and write wal records while we were changing the system to read-write. Now, no
> other backends allow a wal write; UpdateFullPageWrites(), end-of-recovery
> checkpoint, and XLogReportParameters() operations will be performed in the same
> sequence as it is in the startup while changing the system to read-write.

I was looking into the changes espcially recovery related problem, I
have a few questions

1.
+static bool
+XLogAcceptWrites(bool needChkpt, bool bgwriterLaunched,
+ bool localPromoteIsTriggered, XLogReaderState *xlogreader,
+ bool archiveRecoveryRequested, TimeLineID endOfLogTLI,
+ XLogRecPtr endOfLog, TimeLineID thisTimeLineID)
+{
+ bool promoted = false;
+
+ /*
.....
+ if (localPromoteIsTriggered)
{
- checkPointLoc = ControlFile->checkPoint;
+ XLogRecord *record;

...
+ record = ReadCheckpointRecord(xlogreader,
+ ControlFile->checkPoint,
+ 1, false);
if (record != NULL)
{
promoted = true;
...
CreateEndOfRecoveryRecord();
}

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.

>
> 1] http://postgr.es/m/CA+TgmoZ=CCTbAXxMTYZoGXEgqzOz9smkBWrDpsacpjvFcGCuaw@mail.gmail.com
> 2] http://postgr.es/m/CAAJ_b97xX-nqRyM_uXzecpH9aSgoMROrDNhrg1N51fDCDwoy2g@mail.gmail.com

2.
I did not clearly understand your concern in point [2], because of
which you have to postpone RECOVERY_STATE_DONE untill system is set
back to read-write. Can you explain this?

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