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