On Mon, May 13, 2019 at 11:36 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> While I'm kvetching, I can't help noticing that undoinsert.c contains
> functions both for inserting undo and also for reading it, which seems
> like a loose end that needs to be tied up somehow. I'm mildly
> inclined to think that we should rename the file to something more
> generic (e.g. undoaccess.h) rather than splitting it into two files
> (e.g. undoinsert.c and undoread.c).
Changed to undoaccess
Also, it looks to me like you
> need to go through what is currently undoinsert.h and look for stuff
> that can be made private to the .c file. I don't see why thing like
> MAX_PREPARED_UNDO need to be exposed at all,
Ideally, my previous patch should have got rid of MAX_PREPARED_UNDO as
we are now always allocating memory for prepared space but by mistake
I left it in this file. Now, I have removed it.
and for things like
> PreparedUndoSpace it seems like it would suffice to just do 'struct
> PreparedUndoSpace; typedef struct PreparedUndoSpace
> PreparedUndoSpace;' in the header and put the actual 'struct
> PreparedUndoSpace { ... };' definition in the .c file.
Changed, I think
typedef struct PreparedUndoSpace PreparedUndoSpace; in header and
PreparedUndoSpace { ... }; is fine.
And
> UnlockReleaseUndoBuffers has a declaration but no longer has a
> definition, so I think that can go away too.
Removed, and also cleaned some other such declarations.
Pending items to be worked upon:
a) Get rid of UndoRecInfo
b) Get rid of xid in generic undo code and unify epoch and xid to fxid
c) Get rid of discard lock
d) Move log switch related information from transaction header to new
log switch header
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com