Hi,
On Mon, Apr 01, 2024 at 06:05:34AM +0000, Zhijie Hou (Fujitsu) wrote:
> On Monday, April 1, 2024 8:56 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
> Attach the V4 patch which includes the optimization to skip the decoding if
> the snapshot at the syncing restart_lsn is already serialized. It can avoid most
> of the duplicate decoding in my test, and I am doing some more tests locally.
>
Thanks!
1 ===
Same comment as in [1].
In LogicalSlotAdvanceAndCheckReadynessForDecoding(), if we are synchronizing slots
then I think that we can skip:
+ /*
+ * Wait for specified streaming replication standby servers (if any)
+ * to confirm receipt of WAL up to moveto lsn.
+ */
+ WaitForStandbyConfirmation(moveto);
Indeed if we are dealing with synced slot then we know we're in RecoveryInProgress().
Then there is no need to call WaitForStandbyConfirmation() as it could go until
the RecoveryInProgress() in StandbySlotsHaveCaughtup() for nothing (as we already
know it).
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).
Maybe an idea for future improvement (not for now) could be that
SnapBuildSerialize() maintains a "small list" of "already serialized" snapshots.
[1]: https://www.postgresql.org/message-id/ZgayTFIhLfzhpHci%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
