Re: POC: Cleaning up orphaned files using undo logs

On Mon, Jul 22, 2019 at 3:51 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Mon, Jul 22, 2019 at 2:21 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
>
> I have reviewed 0012-Infrastructure-to-execute-pending-undo-actions,
> Please find my comment so far.
..
> 4.
> +void
> +undoaction_redo(XLogReaderState *record)
> +{
> + uint8 info = XLogRecGetInfo(record) & ~XLR_INFO_MASK;
> +
> + switch (info)
> + {
> + case XLOG_UNDO_APPLY_PROGRESS:
> + undo_xlog_apply_progress(record);
> + break;
>
> For HotStandby it doesn't make sense to apply this wal as this
> progress is only required when we try to apply the undo action after
> restart
> but in HotStandby we never apply undo actions.
>

Hmm, I think it is required. Think what if Hotstandby is later
promoted to master and a large part of undo is already applied? In
such a case, we can skip the already applied undo.

> 6.
> + if ((slot == NULL) || (UndoRecPtrGetLogNo(urecptr) != slot->logno))
> + slot = UndoLogGetSlot(UndoRecPtrGetLogNo(urecptr), false);
> +
> + Assert(slot != NULL);
> We are passing missing_ok as false in UndoLogGetSlot. But, not sure
> why we are expecting that undo lot can not be dropped. In multi-log
> transaction it's possible
> that the tablespace in which next undolog is there is already dropped?
>

If the transaction spans multiple logs, then both the logs should be
in the same tablespace. So, how is it possible to drop the tablespace
when part of undo is still pending? AFAICS, the code in
choose_undo_tablespace() doesn't seem to allow switching tablespace
for the same transaction, but I agree if someone used a different
algorithm, then it might be possible.

I think the important question is whether we should allow the same
transactions undo to span across tablespaces? If so, then what you are
telling makes sense and we should handle that, if not, then I think we
are fine here. One might argue that there should be some more strong
checks to ensure that the same transaction will always get the undo
logs from the same tablespace, but I think that is a different thing
then what you are raising here.

Thomas, others, do you have any opinion on this matter?

In FindUndoEndLocationAndSize, there is a check if the next log is
discarded (Case 4: If the transaction is overflowed to ...), won't
this case (considering it is possible) get detected by that check?

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com