On Thu, Jan 29, 2026 at 8:31 PM Amit Kapila <[email protected]> wrote: > > On Fri, Jan 30, 2026 at 9:45 AM shveta malik <[email protected]> wrote: > > > > On Fri, Jan 30, 2026 at 2:15 AM Masahiko Sawada <[email protected]> > > wrote: > > > > > > On Wed, Jan 28, 2026 at 10:06 PM Amit Kapila <[email protected]> > > > wrote: > > > > > > > > On Wed, Jan 28, 2026 at 2:06 AM Masahiko Sawada <[email protected]> > > > > wrote: > > > > > > > > > > I missed fixing one place. Attached the new version. > > > > > > > > > > > > > One question/comment on following change: > > > > + bool use_fast_caught_up_check; > > > > + > > > > + logical_slot_infos_query = > > > > get_old_cluster_logical_slot_infos_query(cluster, > > > > + &use_fast_caught_up_check); > > > > + > > > > upgrade_task_add_step(task, > > > > logical_slot_infos_query, > > > > process_old_cluster_logical_slot_infos, > > > > true, NULL); > > > > + > > > > + /* > > > > + * Check whether slots have consumed all WAL records efficiently by > > > > + * using another query, if not during a live_check. > > > > + */ > > > > + if (use_fast_caught_up_check && !user_opts.live_check) > > > > + { > > > > > > > > Won't this lead to two steps to set caught_up for slots in PG19 and > > > > following versions? If so, is it possible to use just one step even > > > > for PG19 and following versions? > > > > > > Yes, it seems like a good simplification. I've updated the patch > > > accordingly. > > > > > > > At first glance it looks like a simplification, but on closer look, it > > actually makes the code harder to follow and more prone to errors if > > someone modifies it in the future. > > > > I think that is primarily because of the way code is arranged by the > patch. I think it would be better to construct a complete query > separately for fast and non-fast checks. There will be some repeated > parts but the chances of mistakes will be less and it would be easier > to follow.
Agreed. I've updated that function accordingly. > > One minor point: > * Fetch the logical replication slot information. The check whether the > - * slot is considered caught up is done by an upgrade function. This > - * regards the slot as caught up if we don't find any decodable changes. > - * See binary_upgrade_logical_slot_has_caught_up(). > + * slot is considered caught up is done by an upgrade function, unless the > + * fast check is available on the cluster. > > Isn't the caught up check done by an upgrade function both for fast > and non-fast cases? If so, this comment needs to be improved to make > it clear. Fixed. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
v11-0001-pg_upgrade-Optimize-replication-slot-caught-up-c.patch
Description: Binary data
