On 2021-Oct-01, Amit Kapila wrote: > AFAICS, there are two possibilities w.r.t global variables: (a) use > curinsert_flags which we are doing now, (b) another is to introduce a > new global variable, set it after we make the association, and then > reset it after we mark SubTransaction assigned and on error. I have > also thought of passing it via XLogCtlInsert but as that is shared by > different processes, it can be set by one process and be read by > another process which we don't want here.
So, in my mind, curinsert_flags is a way for the high-level user of XLogInsert to pass info about the record being inserted to the low-level xloginsert.c infrastructure. In contrast, XLOG_INCLUDE_XID is being used solely within xloginsert.c, by one piece of low-level infrastructure to communicate to another piece of low-level infrastructure that some cleanup is needed. Nothing outside of xloginsert.c needs to know anything about XLOG_INCLUDE_XID, in contrast with the other bits that can be set by XLogSetRecordFlags. You could move the #define to xloginsert.c and everything would compile fine. Another tell-tale sign that things are not fitting right is that XLOG_INCLUDE_XID is not set via XLogSetRecordFlags, contrary the comment above those defines. (Aside: I wonder why do we have XLogSetRecordFlags at all -- I think we could just pass the other two flags via XLogBeginInsert). Regarding XLOG_INCLUDE_XID, I don't think replacing it with a bit in shared memory is a good idea, since it only applies to the insertion being carried out by the current process, right? I think a straight standalone variable (probably a static boolean in xloginsert.c) might be less confusing. ... so, reading the xact.c code again, TransactionState->assigned really means "whether the subXID-to-topXID association has been wal-logged", which is a completely different meaning from what the term 'assigned' means in all other comments in xact.c ... and I think the subroutine name MarkSubTransactionAssigned() is not a great choice either. -- Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/ "Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
