On Fri, Mar 13, 2026 at 12:43 PM Zhijie Hou (Fujitsu) <[email protected]> wrote: > > On Monday, March 9, 2026 11:13 AM shveta malik <[email protected]> wrote: > > > > No major concerns on 001, just a few trivial things. Do these only if you > > feel > > okay about these. > > > > Thanks for the reviews. I've updated the patch set addressing all comments. >
Thanks Hou-San. Please find my concerns on 001: 1) Consider a case where the page LSN has changed and the sequence has drifted, but the page LSN was not updated because the update interval had not yet elapsed. Later, if there is no further drift for a couple of minutes, we may continue invoking copy_sequence with update_lsn = true. However, since check_seq_privileges_and_drift() keeps returning no drift, the LSN might never get updated. 2) Also, IIUC, we will end up advancing 'next_lsn_update' based on 'update_lsn' even though no actual lsn-update has occurred. As a result, the next page LSN update may never happen if the update_lsn = true cases always coincide with the no-drift case. Shall copy_sequence() call UpdateSubscriptionRelState() even if there is no drift but need_lsn_update is true? thanks Shveta
