On Friday, September 1, 2023 9:05 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@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
