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