Hi, On Wed, Mar 27, 2024 at 09:00:37PM +0530, Bharath Rupireddy wrote: > On Wed, Mar 27, 2024 at 6:54 PM Bertrand Drouvot > <bertranddrouvot...@gmail.com> wrote: > > > > Hi, > > > > On Wed, Mar 27, 2024 at 05:55:05PM +0530, Bharath Rupireddy wrote: > > > On Wed, Mar 27, 2024 at 3:42 PM Bertrand Drouvot > > > Please see the attached v28 patch. > > > > Thanks! > > > > 1 === sorry I missed it in the previous review > > > > if (!(RecoveryInProgress() && slot->data.synced)) > > + { > > now = GetCurrentTimestamp(); > > + update_inactive_since = true; > > + } > > + else > > + update_inactive_since = false; > > > > I think update_inactive_since is not needed, we could rely on (now > 0) > > instead. > > Thought of using it, but, at the expense of readability. I prefer to > use a variable instead.
That's fine too. > However, I changed the variable to be more meaningful to is_slot_being_synced. Yeah makes sense and even easier to read. v29-0001 LGTM. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com