On Thu, Nov 28, 2024 at 5:20 AM Peter Smith <smithpb2...@gmail.com> wrote: > > Hi Nisha. Here are some review comments for patch v51-0002. > > ====== > src/backend/replication/slot.c > > ReplicationSlotAcquire: > > 2. > GENERAL. > > This just is a question/idea. It may not be feasible to change. It > seems like there is a lot of overlap between the error messages in > 'ReplicationSlotAcquire' which are saying "This slot has been > invalidated because...", and with the other function > 'ReportSlotInvalidation' which is kind of the same but called in > different circumstances and with slightly different message text. I > wondered if there is a way to use common code to unify these messages > instead of having a nearly duplicate set of messages for all the > invalidation causes? >
The error handling could be moved to a new function; however, as you pointed out, the contexts in which these functions are called differ. IMO, a single error message may not suit both cases. For example, ReportSlotInvalidation provides additional details and a hint in its message, which isn’t necessary for ReplicationSlotAcquire. Thoughts? -- Thanks, Nisha
