On Mon, Jul 22, 2019 at 3:51 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
>
> On Mon, Jul 22, 2019 at 2:21 PM Amit Kapila <amit.kapil...@gmail.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
