Re: POC: Cleaning up orphaned files using undo logs

On Wed, Jul 24, 2019 at 2:45 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Jul 18, 2019 at 5:10 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> 7.
> +attach_undo_log(UndoLogCategory category, Oid tablespace)
> {
> ..
> if (candidate->meta.tablespace == tablespace)
> + {
> + logno = *place;
> + slot = candidate;
> + *place = candidate->next_free;
> + break;
> + }
>
> Here, the code is breaking from the loop, so why do we need to set
> *place? Am I missing something obvious?
>

I think I know what I was missing. It seems here you are removing an
element from the freelist.

One point related to detach_current_undo_log.

+ LWLockAcquire(&slot->mutex, LW_EXCLUSIVE);
+ slot->pid = InvalidPid;
+ slot->meta.unlogged.xid = InvalidTransactionId;
+ if (full)
+ slot->meta.status = UNDO_LOG_STATUS_FULL;
+ LWLockRelease(&slot->mutex);

If I read the comments in structure UndoLogMetaData, it is mentioned
that 'status' is changed by explicit WAL record whereas there is no
WAL record in code to change the status. I see the problem as well if
we don't WAL log this change. Suppose after changing the status of
this log, we allocate a new log and insert some records in that log as
well for the same transaction for which we have inserted records in
the log which we just marked as FULL. Now, here we form the link
between two logs as the same transaction has overflowed into a new
log. Say, we crash after this. Now, after recovery the log won't be
marked as FULL which means there is a chance that it can be used for
some other transaction, if that happens, then our link for a
transaction spanning to different log will break and we won't be able
to access the data in another log. In short, I think it is important
to WAL log this status change unless I am missing something.

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