Re: prevent immature WAL streaming

On 2021-Sep-15, Kyotaro Horiguchi wrote:

> + 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.

In the new version I removed all this; it was wrong.

> + 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.

You're right, this code is wrong. And in fact I had already noticed it
yesterday, but much to my embarrasment I forgot to fix it. Here is a
fixed version, where I moved the flag set back to AdvanceXLInsertBuffer.
I think doing it anywhere else is going to be very painful. AFAICT we
do the right thing now, but amusingly we don't have any tooling to
verify that the XLP flag is set in the page where we want it.

With this patch we now have two recptrs: the LSN of the broken record,
and the LSN of the missing contrecord. The latter is where to start
writing WAL after recovery is done, and the former is currently unused
but we could use it to double-check that we're aborting (forgetting) the
correct record. I didn't try to implement that, but IIUC it is
xlogreader that would have to do that.

--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"Si quieres ser creativo, aprende el arte de perder el tiempo"