Re: POC: Cleaning up orphaned files using undo logs

Hi Thomas,

I have started reviewing 0003-Add-undo-log-manager, I haven't yet
reviewed but some places I noticed that instead of UndoRecPtr you are
directly
using UndoLogOffset. Which seems like bugs to me

1.
+UndoRecPtr
+UndoLogAllocateInRecovery(UndoLogAllocContext *context,
+ TransactionId xid,
+ uint16 size,
+ bool *need_xact_header,
+ UndoRecPtr *last_xact_start,
....
+ *need_xact_header =
+ context->try_location == InvalidUndoRecPtr &&
+ slot->meta.unlogged.insert == slot->meta.unlogged.this_xact_start;
+ *last_xact_start = slot->meta.unlogged.last_xact_start;

the output parameter last_xact_start is of type UndoRecPtr whereas
slot->meta.unlogged.last_xact_start is of type UndoLogOffset
shouldn't we use MakeUndoRecPtr(logno, offset) here?

2.
+ slot = find_undo_log_slot(logno, false);
+ if (UndoLogOffsetPlusUsableBytes(try_offset, size) <= slot->meta.end)
+ {
+ *need_xact_header = false;
+ return try_offset;
+ }

Here also you are returning directly try_offset instead of UndoRecPtr

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com