Hi,
On Tue, Apr 02, 2024 at 04:24:49AM +0000, Zhijie Hou (Fujitsu) wrote:
> On Monday, April 1, 2024 9:28 PM Bertrand Drouvot
> <bertranddrouvot...@gmail.com> wrote:
> >
> > On Mon, Apr 01, 2024 at 05:04:53PM +0530, Amit Kapila wrote:
> > > On Mon, Apr 1, 2024 at 2:51 PM Bertrand Drouvot
> >
> > >
> > > > 2 ===
> > > >
> > > > + {
> > > > + if (SnapBuildSnapshotExists(remote_slot->restart_lsn))
> > > > + {
> > > >
> > > > That could call SnapBuildSnapshotExists() multiple times for the
> > > > same "restart_lsn" (for example in case of multiple remote slots to
> > > > sync).
> > > >
> > > > What if the sync worker records the last lsn it asks for
> > > > serialization (and serialized ? Then we could check that value first
> > > > before deciding to call (or not)
> > > > SnapBuildSnapshotExists() on it?
> > > >
> > > > It's not ideal because it would record "only the last one" but that
> > > > would be simple enough for now (currently there is only one sync
> > > > worker so that scenario is likely to happen).
> > > >
> > >
> > > Yeah, we could do that but I am not sure how much it can help. I guess
> > > we could do some tests to see if it helps.
> >
> > Yeah not sure either. I just think it can only help and shouldn't make
> > things
> > worst (but could avoid extra SnapBuildSnapshotExists() calls).
>
> Thanks for the idea. I tried some tests based on Nisha's setup[1].
Thank you and Nisha and Shveta for the testing!
> I tried to
> advance the slots on the primary to the same restart_lsn before calling
> sync_replication_slots(), and reduced the data generated by pgbench.
Agree that this scenario makes sense to try to see the impact of
SnapBuildSnapshotExists().
> The SnapBuildSnapshotExists is still not noticeable in the profile.
SnapBuildSnapshotExists() number of calls are probably negligeable when
compared to the IO calls generated by the fast forward logical decoding in this
scenario.
> So, I feel we
> could leave this as a further improvement once we encounter scenarios where
> the duplicate SnapBuildSnapshotExists call becomes noticeable.
Sounds reasonable to me.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
