public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline 3+ messages / 3 participants [nested] [flat]
* [PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline @ 2020-12-09 08:22 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2020-12-09 08:22 UTC (permalink / raw) Since PG13, we should track a timeline switch while sending a historic timeline running physical replication, but WalSndSegmentOpen fails to do that. It is a thinko of 709d003fbd. --- src/backend/replication/walsender.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index fe0d368a35..8545c6c423 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2491,7 +2491,7 @@ WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo, XLogSegNo endSegNo; XLByteToSeg(sendTimeLineValidUpto, endSegNo, state->segcxt.ws_segsize); - if (state->seg.ws_segno == endSegNo) + if (nextSegNo == endSegNo) *tli_p = sendTimeLineNextTLI; } -- 2.27.0 ----Next_Part(Mon_Jan__4_12_06_10_2021_267)---- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message @ 2022-02-03 06:50 Kyotaro Horiguchi <[email protected]> 2022-02-04 01:59 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Kyotaro Horiguchi @ 2022-02-03 06:50 UTC (permalink / raw) To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] At Thu, 3 Feb 2022 13:59:03 +0900, Fujii Masao <[email protected]> wrote in > > > On 2022/02/02 23:46, Bharath Rupireddy wrote: > > On Tue, Feb 1, 2022 at 9:39 PM Fujii Masao > > <[email protected]> wrote: > >> I found that CreateRestartPoint() already reported the redo lsn as > >> follows after emitting the restartpoint log message. To avoid > >> duplicated logging of the same information, we should update this > >> code? > >> > >> ereport((log_checkpoints ? LOG : DEBUG2), > >> (errmsg("recovery restart point at %X/%X", > >> LSN_FORMAT_ARGS(lastCheckPoint.redo)), > >> xtime ? errdetail("Last completed transaction was > >> at log time %s.", > >> timestamptz_to_str(xtime)) > >> : 0)); > >> > >> This code reports lastCheckPoint.redo as redo lsn. OTOH, with the > >> patch, LogCheckpointEnd() reports > >> ControlFile->checkPointCopy.redo. They may be different, for example, > >> when the current DB state is not DB_IN_ARCHIVE_RECOVERY. In this case, > >> which lsn should we report as redo lsn? > > Do we ever reach CreateRestartPoint when ControlFile->stat != > > DB_IN_ARCHIVE_RECOVERY? Assert(ControlFile->state == > > DB_IN_ARCHIVE_RECOVERY); in CreateRestartPoint doesn't fail any > > regression tests. > > ISTM that CreateRestartPoint() can reach the condition > ControlFile->state != DB_IN_ARCHIVE_RECOVERY. Please imagine the case > where CreateRestartPoint() has already started and calls > CheckPointGuts(). If the standby server is promoted while > CreateRestartPoint() is flushing the data to disk at CheckPointGuts(), > the state would be updated to DB_IN_PRODUCTION and > CreateRestartPoint() can see the state != DB_IN_ARCHIVE_RECOVERY > later. By the way a comment there: > * this is a quick hack to make sure nothing really bad happens if somehow > * we get here after the end-of-recovery checkpoint. now looks a bit wrong since now it's normal that a restartpoint ends after promotion. > As far as I read the code, this case seems to be able to make the > server unrecoverable. If this case happens, since pg_control is not > updated, pg_control still indicates the REDO LSN of last valid > restartpoint. But CreateRestartPoint() seems to delete old WAL files > based on its "current" REDO LSN not pg_control's REDO LSN. That is, > WAL files required for the crash recovery starting from pg_control's > REDO LSN would be removed. Seems right. (I didn't confirm the behavior, though ) > If this understanding is right, to address this issue, probably we > need to make CreateRestartPoint() do nothing (return immediately) when > the state != DB_IN_ARCHIVE_RECOVERY? On way to take. In that case should we log something like "Restartpoint canceled" or something? By the way, restart point should start only while recoverying, and at the timeof the start both checkpoint.redo and checkpoint LSN are already past. We shouldn't update minRecovery point after promotion, but is there any reason for not updating the checkPoint and checkPointCopy? If we update them after promotion, the which-LSN-to-show problem would be gone. regards. -- Kyotaro Horiguchi NTT Open Source Software Center ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message 2022-02-03 06:50 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]> @ 2022-02-04 01:59 ` Fujii Masao <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Fujii Masao @ 2022-02-04 01:59 UTC (permalink / raw) To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] On 2022/02/03 15:50, Kyotaro Horiguchi wrote: > On way to take. In that case should we log something like > "Restartpoint canceled" or something? +1 > By the way, restart point should start only while recoverying, and at > the timeof the start both checkpoint.redo and checkpoint LSN are > already past. We shouldn't update minRecovery point after promotion, > but is there any reason for not updating the checkPoint and > checkPointCopy? If we update them after promotion, the > which-LSN-to-show problem would be gone. I tried to find the reason by reading the past discussion, but have not found that yet. If we update checkpoint and REDO LSN at pg_control in that case, we also need to update min recovery point at pg_control? Otherwise the min recovery point at pg_control still indicates the old LSN that previous restart point set. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-04 01:59 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-09 08:22 [PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline Kyotaro Horiguchi <[email protected]> 2022-02-03 06:50 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]> 2022-02-04 01:59 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox