Re: [PoC] pg_upgrade: allow to upgrade publisher node
От | Peter Smith |
---|---|
Тема | Re: [PoC] pg_upgrade: allow to upgrade publisher node |
Дата | |
Msg-id | CAHut+Pu3765fuTSrRUUhZtuRVQCU3NP+XYQ9Rrj=qXW6hAxOQw@mail.gmail.com обсуждение исходный текст |
Ответ на | RE: [PoC] pg_upgrade: allow to upgrade publisher node ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>) |
Ответы |
RE: [PoC] pg_upgrade: allow to upgrade publisher node
Re: [PoC] pg_upgrade: allow to upgrade publisher node |
Список | pgsql-hackers |
Hi, here are my review comments for v26-0003 It seems I must defend some of my previous suggestions from v25* [1], so here goes... ====== src/bin/pg_upgrade/check.c 1. check_and_dump_old_cluster CURRENT CODE (with v26-0003 patch applied) /* Extract a list of logical replication slots */ get_old_cluster_logical_slot_infos(); ... /* * Logical replication slots can be migrated since PG17. See comments atop * get_old_cluster_logical_slot_infos(). */ if (GET_MAJOR_VERSION(old_cluster.major_version) >= 1700) { check_old_cluster_for_lost_slots(); /* * Do additional checks if a live check is not required. This requires * that confirmed_flush_lsn of all the slots is the same as the latest * checkpoint location, but it would be satisfied only when the server * has been shut down. */ if (!live_check) check_old_cluster_for_confirmed_flush_lsn(); } SUGGESTION /* * Logical replication slots can be migrated since PG17. See comments atop * get_old_cluster_logical_slot_infos(). */ if (GET_MAJOR_VERSION(old_cluster.major_version) >= 1700) // NOTE 1a. { /* Extract a list of logical replication slots */ get_old_cluster_logical_slot_infos(); if (count_old_cluster_slots()) // NOTE 1b. { check_old_cluster_for_lost_slots(); /* * Do additional checks if a live check is not required. This requires * that confirmed_flush_lsn of all the slots is the same as the latest * checkpoint location, but it would be satisfied only when the server * has been shut down. */ if (!live_check) check_old_cluster_for_confirmed_flush_lsn(); } } ~~ Benefits: 1a. One version check instead of multiple. ~ 1b. Upfront slot counting means - only call 1 time to count_old_cluster_slots(). - unnecessary calls to other check* functions are avoided ~ 1c. get_old_cluster_logical_slot_infos - No version check is needed. check_old_cluster_for_lost_slots - Call to count_old_cluster_slots is not needed - Quick exit not needed. check_old_cluster_for_confirmed_flush_lsn - Call to count_old_cluster_slots is not needed - Quick exit not needed. ~~~ 2. check_old_cluster_for_lost_slots + /* Quick exit if the cluster does not have logical slots. */ + if (count_old_cluster_logical_slots() == 0) + return; Refer [1]#4. Can remove this because #1b above. ~~~ 3. check_old_cluster_for_confirmed_flush_lsn + /* Quick exit if the cluster does not have logical slots. */ + if (count_old_cluster_logical_slots() == 0) + return; Refer [1]#5. Can remove this because #1b above. ~~~ 4. .../t/003_logical_replication_slots.pl /shipped/replicated/ Kuroda-san 26/8 wrote: You meant to say s/replicated/shipped/, right? Fixed. No, I meant what I wrote for [1]#7. I was referring to the word "shipped" in the message 'check changes are shipped to the subscriber'. Now there are 2 places to change instead of one. ------ [1] my review of v25-0003. https://www.postgresql.org/message-id/CAHut%2BPsdkhcVG5GY4ZW0DMUF8FG%3DWvjaGN%2BNA4XFLrzxWSQXVA%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia
В списке pgsql-hackers по дате отправления:
Следующее
От: Michael PaquierДата:
Сообщение: Re: Autogenerate some wait events code and documentation