Some review comments for patch v61-0002

======
src/backend/replication/slot.c

1.
  * Whether a slot needs to be invalidated depends on the cause. A slot is
- * removed if it:
+ * invalidated if it:
  * - RS_INVAL_WAL_REMOVED: requires a LSN older than the given segment
  * - RS_INVAL_HORIZON: requires a snapshot <= the given horizon in the given
  *   db; dboid may be InvalidOid for shared relations
  * - RS_INVAL_WAL_LEVEL: is logical
+ * - RS_INVAL_IDLE_TIMEOUT: idle slot timeout has occurred

1a.
Firstly the wording seems odd. "Is invalidated if it:" (missing words?)

~

1b.
Secondly, is this comment strictly correct? IIUC it's not *always*
going to be invalidated just because the cause is one of those listed.
e.g. the code calls InvalidatePossiblyObsoleteSlot but it might not
end up invalidating the slot having a cause RS_INVAL_IDLE_TIMEOUT.

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


Reply via email to