Sorry for the long delay...
On Thu, Jul 4, 2019 at 5:25 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Thu, Jul 4, 2019 at 9:45 AM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Wed, Jul 03, 2019 at 08:23:44PM +0200, Julien Rouhaud wrote:
> > > So the patch compiles and works as intended. I don't have much to say
> > > about it, it all looks good to me, since the concerns about xactdesc.c
> > > aren't worth the trouble.
> > >
> > > I'm not sure that I understand Michael's objection though, as
> > > xl_xact_prepare is not a new definition and AFAICS it couldn't contain
> > > the records anyway. So I'll let him say if he has further objections
> > > or if it's ready for committer!
> >
> > This patch provides parsing information only for the header of the 2PC
> > record. Wouldn't it be interesting to get more information from the
> > various TwoPhaseRecordOnDisk's callbacks? We could also print much
> > more information in xact_desc_prepare(). Like the subxacts, the XID,
> > the invalidation messages and the delete-on-abort/commit rels.
>
> Most of those are already described in the COMMIT PREPARE message,
> wouldn't that be redundant? abortrels aren't displayed anywhere
> though, so +1 for adding them.
xact_desc_abort() for ROLLBACK PREPARED describes abortrels. No?
> I also see that the dbid isn't displayed in any of the 2PC message,
> that'd be useful to have it directly instead of looking for it in
> other messages for the same transaction.
dbid is not reported even in COMMIT message. So I don't like adding
dbid into only the PREPARE message.
Regards,
--
Fujii Masao