Re: Synchronizing slots from primary to standby

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Ajin Cherian <itsajin(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Subject: Re: Synchronizing slots from primary to standby
Date: 2024-01-17 04:33:10
Message-ID: CAHut+PuECB8fNBfXMdTHSMKF9kL=0XqPw1Am4NVahfJSSHzoYg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here is a review comment for the latest v62-0002 changes.

======
src/backend/replication/logical/slotsync.c

1.
+ if (namestrcmp(&slot->data.plugin, remote_slot->plugin) == 0 &&
+ slot->data.database == dbid &&
+ remote_slot->restart_lsn == slot->data.restart_lsn &&
+ remote_slot->catalog_xmin == slot->data.catalog_xmin &&
+ remote_slot->two_phase == slot->data.two_phase &&
+ remote_slot->failover == slot->data.failover &&
+ remote_slot->confirmed_lsn == slot->data.confirmed_flush)
+ return false;

For consistency, I think it would be better to always code the remote
slot value on the LHS and the local slot value on the RHS, instead of
the current random mix.

And rename 'dbid' to 'remote_dbid' for name consistency too.

SUGGESTION
if (namestrcmp(remote_slot->plugin, &slot->data.plugin) == 0 &&
remote_dbid == slot->data.database &&
remote_slot->restart_lsn == slot->data.restart_lsn &&
remote_slot->catalog_xmin == slot->data.catalog_xmin &&
remote_slot->two_phase == slot->data.two_phase &&
remote_slot->failover == slot->data.failover &&
remote_slot->confirmed_lsn == slot->data.confirmed_flush)
return false;

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2024-01-17 04:37:14 Re: Synchronizing slots from primary to standby
Previous Message Jonathan S. Katz 2024-01-17 04:24:49 Re: heavily contended lwlocks with long wait queues scale badly