On Mon, May 26, 2025 at 12:46 PM Zhijie Hou (Fujitsu)
<houzj.f...@fujitsu.com> wrote:
>
> Attaching the V32 patch set which addressed comments in [1]~[5].
Thanks for the patch, I am still reviewing the patches, please find
few trivial comments for patch001:
1)
+ FullTransactionId last_phase_at; /* publisher transaction ID that must
+ * be awaited to complete before
+ * entering the final phase
+ * (RCI_WAIT_FOR_LOCAL_FLUSH) */
'last_phase_at' seems like we are talking about the phase in the past.
(similar to 'last' in last_recv_time).
Perhaps we should name it as 'final_phase_at'
2)
RetainConflictInfoData data = {0};
We can change this name as well to rci_data.
3)
+ /*
+ * Compute FullTransactionId for the oldest running transaction ID. This
+ * handles the case where transaction ID wraparound has occurred.
+ */
+ full_xid = FullTransactionIdFromAllowableAt(next_full_xid,
oldest_running_xid);
Shall we name it to full_oldest_xid for better clarity?
4)
+ /*
+ * Update and check the remote flush position if we are applying changes
+ * in a loop. This is done at most once per WalWriterDelay to avoid
+ * performing costy operations in get_flush_position() too frequently
+ * during change application.
+ */
+ if (last_flushpos < rci_data->remote_lsn && rci_data->last_recv_time &&
+ TimestampDifferenceExceeds(rci_data->flushpos_update_time,
+ rci_data->last_recv_time, WalWriterDelay))
a) costy --> costly
thanks
Shveta
