On Wed, Jan 14, 2026 at 5:47 PM Chao Li <[email protected]> wrote: > > > > > On Jan 14, 2026, at 09:26, Fujii Masao <[email protected]> wrote: > > > > On Mon, Jan 12, 2026 at 4:08 PM Chao Li <[email protected]> wrote: > >> Thanks for the patch. Here are my comments on v4. > > > > Thanks for the review! > > > > > >> 1 - 0001 > >> ``` > >> + /* > >> + * Save the last flushed position as the replication start point. > >> On > >> + * reconnect, replication resumes from there to avoid re-sending > >> flushed > >> + * data. > >> + */ > >> + startpos = output_fsync_lsn; > >> ``` > >> > >> Looking at function OutputFsync(), fsync() may fail and there a few > >> branches to return early without fsync(), so should we only update > >> startpos after fsync()? > > > > Maybe not, but I might be missing something. Could you clarify what > > concrete scenario would be problematic with the current code? > > > > I just reviewed the patch again, and I think I was wrong wrt this comment: > > * If fsync() fails, the process will fail out, no reconnect will happen, so > wether or not updating startpos doesn’t matter; > * if (fsync_interval <= 0), fsync is not required, but we still need to > update startpos > * if (!output_needs_fsync), meaning nothing new to fsync, but we still need > to update startpos if startpos has not been updated > > So, I withdraw this comment. > > V5 LGTM.
Thanks for the review! I've pushed the patches. Regards, -- Fujii Masao
