On Wed, Feb 5, 2025 at 5:53 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Some review comments for v2-0001. > > ====== > doc/src/sgml/system-views.sgml > > 1. > The time when the slot became inactive. NULL if the slot is currently > being streamed. If the slot becomes invalid, this value will never be > updated. Note that for slots on the standby that are being synced from > a primary server (whose synced field is true), the inactive_since > indicates the time when slot synchronization (see Section 47.2.3) was > most recently stopped. NULL if the slot has always been synchronized. > On standby, this is useful for slots that are being synced from a > primary server (whose synced field is true) so they know when the slot > stopped being synchronized. > > ~ > > (maybe not strictly related to this patch, but perhaps you can fix it > in passing because it will help the readability of the newly added > sentence also...) > > There are 2 different explanations for NULL: > "NULL if the slot is currently being streamed." > "NULL if the slot has always been synchronized." > > I'm assuming that 2nd description is only to be read in the scope of > "Note that for slots on the standby that are being synced from a > primary server...". IMO inserting a blank line before "Note that for > slots on the standby..." will help separate these two quite different > descriptions for the same field. >
This is unrelated to this patch, but I don't mind you proposing a separate patch if you feel it will make it clear. Did you see separate paragraphs in other descriptions? -- With Regards, Amit Kapila.