On Friday, September 1, 2023 9:05 PM Kuroda, Hayato/黒田 隼人 <kuroda.hay...@fujitsu.com> wrote: >
Hi, Thanks for updating the patch. I have a comment about the check related to the wal_status. Currently, there are few places where we check the wal_status of slots. e.g. check_old_cluster_for_valid_slots(),get_loadable_libraries(), and get_old_cluster_logical_slot_infos(). But as discussed in another thread[1]. There are some kind of WALs that will be written when pg_upgrade are checking the old cluster which could cause the wal size to exceed the max_slot_wal_keep_size. In this case, checkpoint will remove the wals required by slots and invalidate these slots(the wal_status get changed as well). Based on this, it’s possible that the slots we get each time when checking wal_status are different, because they may get changed in between these checks. This may not cause serious problems for now, because we will either copy all the slots including ones invalidated when upgrading or we report ERROR. But I feel it's better to get consistent result each time we check the slots to close the possibility for problems in the future. So, I feel we could centralize the check for wal_status and slots fetch, so that even if some slots status changed after that, it won't have a risk to affect our check. What do you think ? [1] https://www.postgresql.org/message-id/flat/CAA4eK1LLik2818uzYqS73O%2BHe5LK_%2B%3DkthyZ6hwT6oe9TuxycA%40mail.gmail.com#16efea0a76d623b1335e73fc1e28f5ef Best Regards, Hou zj
