On Wed, Jul 24, 2019 at 2:45 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jul 18, 2019 at 5:10 PM Amit Kapila <amit.kapil...@gmail.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