Please find few more comments: 1) ProcessStandbyPSRequestMessage: + /* + * This shouldn't happen because we don't support getting primary status + * message from standby. + */ + if (RecoveryInProgress()) + elog(ERROR, "the primary status is unavailable during recovery");
a) This error is not clear. Is it supposed to be user oriented error or internal error? As a user, it is difficult to interpret this error and take some action. b) What I understood is that there is no user of enabling 'retain_conflict_info' for a subscription which is subscribing to a publisher which is physical standby too. So shall we emit such an ERROR during 'Create Sub(retain_conflict_info=on)' itself? But that would need checking whether the publisher is physical standby or not during CREATE-SUB. Is that a possibility? 2) ---------- send_feedback(last_received, requestReply, requestReply); + maybe_advance_nonremovable_xid(&data, false); + /* * Force reporting to ensure long idle periods don't lead to * arbitrarily delayed stats. Stats can only be reported outside ---------- Why do we need this call to 'maybe_advance_nonremovable_xid' towards end of LogicalRepApplyLoop() i.e. the last call? Can it make any further 'data.phase' change here? IIUC, there are 2 triggers for 'data.phase' change through LogicalRepApplyLoop(). First one is for the very first time when we start this loop, it will set data.phase to 0 (RCI_GET_CANDIDATE_XID) and will call maybe_advance_nonremovable_xid(). After that, LogicalRepApplyLoop() function can trigger a 'data.phase' change only when it receives a response from the publisher. Shouldn't the first 4 calls to maybe_advance_nonremovable_xid() from LogicalRepApplyLoop() suffice? 3) Code is almost the same for GetOldestActiveTransactionId() and GetOldestTransactionIdInCommit(). I think we can merge these two. GetOldestActiveTransactionId() can take new arg "getTxnInCommit". GetOldestTransactionIdInCommit() can call GetOldestActiveTransactionId() with that arg as true, whereas other 2 callers can pass it as false. thanks Shveta
