From dae203ef692415428977d49bdf116c3a5fed344e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arpad=20M=C3=BCller?= Date: Thu, 5 Jun 2025 01:02:31 +0200 Subject: [PATCH] pgxn: support generations in safekeepers_cmp (#12129) `safekeepers_cmp` was added by #8840 to make changes of the safekeeper set order independent: a `sk1,sk2,sk3` specifier changed to `sk3,sk1,sk2` should not cause a walproposer restart. However, this check didn't support generations, in the sense that it would see the `g#123:` as part of the first safekeeper in the list, and if the first safekeeper changes, it would also restart the walproposer. Therefore, parse the generation properly and make it not be a part of the generation. This PR doesn't add a specific test, but I have confirmed locally that `test_safekeepers_reconfigure_reorder` is fixed with the changes of PR #11712 applied thanks to this PR. Part of https://github.com/neondatabase/neon/issues/11670 --- pgxn/neon/walproposer.c | 4 ++++ pgxn/neon/walproposer_pg.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/pgxn/neon/walproposer.c b/pgxn/neon/walproposer.c index f42103c7cd..c63edb1398 100644 --- a/pgxn/neon/walproposer.c +++ b/pgxn/neon/walproposer.c @@ -119,6 +119,10 @@ WalProposerCreate(WalProposerConfig *config, walproposer_api api) { wp_log(FATAL, "failed to parse neon.safekeepers generation number: %m"); } + if (*endptr != ':') + { + wp_log(FATAL, "failed to parse neon.safekeepers: no colon after generation"); + } /* Skip past : to the first hostname. */ host = endptr + 1; } diff --git a/pgxn/neon/walproposer_pg.c b/pgxn/neon/walproposer_pg.c index d15bf91d24..c90702a282 100644 --- a/pgxn/neon/walproposer_pg.c +++ b/pgxn/neon/walproposer_pg.c @@ -272,6 +272,30 @@ split_safekeepers_list(char *safekeepers_list, char *safekeepers[]) return n_safekeepers; } +static char *split_off_safekeepers_generation(char *safekeepers_list, uint32 *generation) +{ + char *endptr; + + if (strncmp(safekeepers_list, "g#", 2) != 0) + { + return safekeepers_list; + } + else + { + errno = 0; + *generation = strtoul(safekeepers_list + 2, &endptr, 10); + if (errno != 0) + { + wp_log(FATAL, "failed to parse neon.safekeepers generation number: %m"); + } + if (*endptr != ':') + { + wp_log(FATAL, "failed to parse neon.safekeepers: no colon after generation"); + } + return endptr + 1; + } +} + /* * Accept two coma-separated strings with list of safekeeper host:port addresses. * Split them into arrays and return false if two sets do not match, ignoring the order. @@ -283,6 +307,16 @@ safekeepers_cmp(char *old, char *new) char *safekeepers_new[MAX_SAFEKEEPERS]; int len_old = 0; int len_new = 0; + uint32 gen_old = INVALID_GENERATION; + uint32 gen_new = INVALID_GENERATION; + + old = split_off_safekeepers_generation(old, &gen_old); + new = split_off_safekeepers_generation(new, &gen_new); + + if (gen_old != gen_new) + { + return false; + } len_old = split_safekeepers_list(old, safekeepers_old); len_new = split_safekeepers_list(new, safekeepers_new);