On Wed, Mar 27, 2024 at 2:55 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > On Wed, Mar 27, 2024 at 11:39 AM shveta malik <shveta.ma...@gmail.com> wrote: > > > > Thanks for the patch. Few trivial things: > > Thanks for reviewing. > > > ---------- > > 1) > > system-views.sgml: > > > > a) "Note that the slots" --> "Note that the slots on the standbys," > > --it is good to mention "standbys" as synced could be true on primary > > as well (promoted standby) > > Done. > > > b) If you plan to add more info which Bertrand suggested, then it will > > be better to make a <note> section instead of using "Note" > > I added the note that Bertrand specified upthread. But, I couldn't > find an instance of adding <note> ... </note> within a table. Hence > with "Note that ...." statments just like any other notes in the > system-views.sgml. pg_replication_slot in system-vews.sgml renders as > table, so having <note> ... </note> may not be a great idea. > > > 2) > > commit msg: > > > > "The impact of this > > on a promoted standby inactive_since is always NULL for all > > synced slots even after server restart. > > " > > Sentence looks broken. > > --------- > > Reworded. > > > Apart from the above trivial things, v26-001 looks good to me. > > Please check the attached v27 patch which also has Bertrand's comment > on deduplicating the TAP function. I've now moved it to Cluster.pm. >
Thanks for the patch. Regarding doc, I have few comments. + Note that the slots on the standbys that are being synced from a + primary server (whose <structfield>synced</structfield> field is + <literal>true</literal>), will get the + <structfield>inactive_since</structfield> value from the + corresponding remote slot on the primary. Also, note that for the + synced slots on the standby, after the standby starts up (i.e. after + the slots are loaded from the disk), the inactive_since will remain + zero until the next slot sync cycle. a) "inactive_since will remain zero" Since it is user exposed info and the user finds it NULL in pg_replication_slots, shall we mention NULL instead of 0? b) Since we are referring to the sync cycle here, I feel it will be good to give a link to that page. + zero until the next slot sync cycle (see + <xref linkend="logicaldecoding-replication-slots-synchronization"/> for + slot synchronization details). thanks Shveta