On Thu, Nov 14, 2024 at 12:12 PM Nisha Moond <nisha.moond...@gmail.com> wrote: > > On Thu, Oct 31, 2024 at 11:05 PM Bruce Momjian <br...@momjian.us> wrote: > > > > > > Yes, all good suggestions, updated patch attached. > > > > Few comments for the changes under "inactive_since" description: > > + The time when slot synchronization (see <xref > + linkend="logicaldecoding-replication-slots-synchronization"/>) > + was most recently stopped. <literal>NULL</literal> if the slot > + has always been synchronized. This is useful for slots on the > + standby that are being synced from a primary server (whose > + <structfield>synced</structfield> field is <literal>true</literal>) > + so they know when the slot stopped being synchronized. > > 1) > To me, the above lines give the impression that "inactive_since" is > only meaningful for the logical slots which are being synchronized > from primary to standby, which is not correct. On a primary node, this > column gives the timestamp when any slot becomes inactive. By removing > the line - > - The time since the slot has become inactive. > I feel we lost the description that explains this column’s purpose on > primary nodes. I suggest explicitly clarifying the inactive_since > column's meaning on primary nodes as well. >
Good point. The same holds true for following change in the patch as well: - /* The time since the slot has become inactive */ + /* The time when slot synchronization was most recently stopped. */ TimestampTz inactive_since; Do you have suggestions for improving the proposal? > 2) Can we add more details to it for column's behavior on restarting > a node, something like - > "For the inactive slots, restarting a node resets the "inactive_since" > time to the node's start time. " > I am not so sure about this. Adding too-long descriptions also sometimes confuses users. -- With Regards, Amit Kapila.
