Horton

Re: prevent immature WAL streaming

Kyotaro Horiguchi <horikyota.ntt@gmail.com>

View thread View on postgr.es
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To: alvherre@alvh.no-ip.org
Cc: bossartn@amazon.com, andres@anarazel.de, masao.fujii@oss.nttdata.com, pgsql-hackers@lists.postgresql.org, mengjuan.cmj@alibaba-inc.com, Jakub.Wartak@tomtom.com
Date: 2021-09-15T03:00:56Z
Lists: pgsql-hackers 
At Tue, 14 Sep 2021 22:32:04 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in 
> On 2021-Sep-14, Alvaro Herrera wrote:
> 
> > On 2021-Sep-08, Kyotaro Horiguchi wrote:
> > 
> > > Thanks!  As my understanding the new record add the ability to
> > > cross-check between a teard-off contrecord and the new record inserted
> > > after the teard-off record.  I didn't test the version by myself but
> > > the previous version implemented the essential machinery and that
> > > won't change fundamentally by the new record.
> > > 
> > > So I think the current patch deserves to see the algorithm actually
> > > works against the problem.
> > 
> > Here's a version with the new record type.  It passes check-world, and
> > it seems to work correctly to prevent overwrite of the tail end of a
> > segment containing a broken record.  This is very much WIP still;
> > comments are missing and I haven't tried to implement any sort of
> > verification that the record being aborted is the right one.
> 
> Here's the attachment I forgot earlier.

(I missed the chance to complain about that:p)

Tnaks for the patch!

-		CopyXLogRecordToWAL(rechdr->xl_tot_len, isLogSwitch, rdata,
-							StartPos, EndPos);
+		CopyXLogRecordToWAL(rechdr->xl_tot_len, isLogSwitch,
+							flags & XLOG_SET_ABORTED_PARTIAL,
+							rdata, StartPos, EndPos);

The new xlog flag XLOG_SET_ABORTED_PARTIAL is used only by
RM_XLOG_ID/XLOG_OVERWRITE_CONTRECORD records, so the flag value is the
equivalent of the record type.  We might instead want a new flag
XLOG_SPECIAL_TREATED_RECORD or something to quickly distinguish
records that need a special treat like XLOG_SWITCH.

  if (flags & XLOG_SPECIAL_TREATED_RECORD)
  {
  	if (rechdr->xl_rmid == RM_XLOG_ID)
	{
       if (info ==	XLOG_SWITCH)
	      isLogSwitch = true;
       if (info == XLOG_OVERWRITE_CONTRECORD)
	      isOverwrite = true;
    }
  }
  ..
  CopyXLogRecrodToWAL(.., isLogSwitch, isOverwrite, rdata, StartPos, EndPos);


+			/* XXX can only happen once in the loop. Verify? */
+			if (set_aborted_partial)
+				pagehdr->xlp_info |= XLP_FIRST_IS_ABORTED_PARTIAL;
+

I'm not sure about the reason for the change from the previous patch
(I might be missing something), this sets the flag on the *next* page
of the page where the record starts.  So in the first place we
shouldn't set the flag there.  The page header flags of the first page
is set by AdvanceXLInsertBuffer. If we want to set the flag in the
function, we need to find the page header for the beginning of the
record and make sure that the record is placed at the beginning of the
page.  (It is the reason that I did that in AdvanceXLInsertBuffer..)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Commits

  1. Fix determination of broken LSN in OVERWRITTEN_CONTRECORD

  2. Fix instability in 026_overwrite_contrecord.pl test.

  3. Fix typo

  4. Change recently added test code for stability

  5. Remove unstable, unnecessary test; fix typo

  6. Repair two portability oversights of new test

  7. Fix WAL replay in presence of an incomplete record

  8. Document XLOG_INCLUDE_XID a little better

  9. Revert "Avoid creating archive status ".ready" files too early"

  10. Avoid creating archive status ".ready" files too early

  11. Fix scenario where streaming standby gets stuck at a continuation record.

  12. Teach xlogreader to follow timeline switches

  13. Forbid using pg_xlogfile_name() and pg_xlogfile_name_offset() during