Re: Add --system-identifier / -s option to pg_resetwal
От | Fujii Masao |
---|---|
Тема | Re: Add --system-identifier / -s option to pg_resetwal |
Дата | |
Msg-id | a71e83c0-da65-40eb-943e-bfee1f9f7ffb@oss.nttdata.com обсуждение исходный текст |
Ответ на | Re: Add --system-identifier / -s option to pg_resetwal (Nikolay Samokhvalov <nik@postgres.ai>) |
Список | pgsql-hackers |
On 2025/06/05 3:46, Nikolay Samokhvalov wrote: > Thank you, Peter and Michael, for the reviews > > Attached is v2, simplified as suggested: > - Removed short option -s > - Removed interactive confirmation and --force > - Simplified tests leaving only essential ones In my environment, the test failed with the following output: # Failed test 'system identifier was changed correctly' # at t/001_basic.pl line 203. # got: '-8570200862721897295' # expected: '9876543210987654321' # Looks like you failed 1 test of 84. t/001_basic.pl ...... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/84 subtests I think this happens because the system_identifier returned by pg_control_system() is a bigint, not an unsigned one. So either we need to use a different method to retrieve the value correctly, or we should fix pg_control_system() to report the system identifier properly (though that would be a separate issue). + if (set_sysid != 0) + { + printf(_("System identifier: " UINT64_FORMAT "\n"), + ControlFile.system_identifier); + } For consistency with PrintControlValues() and pg_controldata, the label should be "Database system identifier", and we should use PRIu64 instead of UINT64_FORMAT for gettext()? + {"system-identifier", required_argument, NULL, 3}, {"next-transaction-id", required_argument, NULL, 'x'}, {"wal-segsize", required_argument, NULL, 1}, {"char-signedness", required_argument, NULL, 2}, It would be more consistent to place the "system-identifier" entry just after "char-signedness". One question about this feature: why do we need to allow explicitly setting the system identifier? If the goal is simply to ensure it's different from the original, wouldn't it be sufficient to let pg_resetwal generate a new (hopefully unique) value using existing logic, like what's done in GuessControlValues() or BootStrapXLOG()? Regards, -- Fujii Masao NTT DATA Japan Corporation
В списке pgsql-hackers по дате отправления: