This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 33e08725fb447dae7309af5d1b34e0a75c6badc0 Author: Violet Cheng <[email protected]> AuthorDate: Mon Oct 31 07:04:54 2022 +0800 fix gp_gettmid to return correct startup timestamp. (#14204) Previously, gp_gettmid() returns PgStartTime as the start time of gpdb ("tmid") and saves it into instrument shared memory, but the value is wrong after converting to `TimestampTz` (which is int64). Since the value is saved in the shared memory, plus GPCC has been using 32-bit integer for tmid, adapting to int64 will require a lot of change in both GPDB's and GPCC's coed. Therefore, in this commit, gp_gettmid() converts PgStartTime to a correct int32 value instead. Co-authored-by: Violet Cheng <[email protected]> --- src/backend/executor/instrument.c | 24 ++++++++++- src/backend/executor/test/Makefile | 12 ++++++ src/backend/executor/test/instrument_test.c | 64 +++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c index efac4315eb..ea879180e6 100644 --- a/src/backend/executor/instrument.c +++ b/src/backend/executor/instrument.c @@ -536,9 +536,31 @@ instrShmemRecycleCallback(ResourceReleasePhase phase, bool isCommit, bool isTopL SpinLockRelease(&InstrumentGlobal->lock); } +/* + * Cast PgStartTime from TimestampTz to int32. Separated from gp_gettmid() to avoid elog() in + * gp_gettmid() to cause panic when running unit tests. + * Return -1 for invalid PgStartTime or overflow values. + */ +static int32 gp_gettmid_helper() +{ + pg_time_t time; + if (PgStartTime < 0) + return -1; + time = timestamptz_to_time_t(PgStartTime); + if (time > INT32_MAX) + return -1; + return (int32)time; +} + +/* + * Wrapper for gp_gettmid_helper() + */ static void gp_gettmid(int32* tmid) { - *tmid = (int32) PgStartTime; + int32 time = gp_gettmid_helper(); + if (time == -1) + elog(PANIC, "time_t converted from PgStartTime is too large"); + *tmid = time; } /* helper functions for WAL usage accumulation */ diff --git a/src/backend/executor/test/Makefile b/src/backend/executor/test/Makefile new file mode 100644 index 0000000000..b8480f4c17 --- /dev/null +++ b/src/backend/executor/test/Makefile @@ -0,0 +1,12 @@ +subdir=src/backend/executor +top_builddir=../../../.. +include $(top_builddir)/src/Makefile.global + +TARGETS= instrument + +include $(top_srcdir)/src/backend/mock.mk + +instrument.t: $(MOCK_DIR)/backend/access/transam/distributedlog_mock.o \ + $(MOCK_DIR)/backend/access/hash/hash_mock.o \ + $(MOCK_DIR)/backend/utils/fmgr/fmgr_mock.o + diff --git a/src/backend/executor/test/instrument_test.c b/src/backend/executor/test/instrument_test.c new file mode 100644 index 0000000000..282824b8dd --- /dev/null +++ b/src/backend/executor/test/instrument_test.c @@ -0,0 +1,64 @@ +#include <stdarg.h> +#include <stddef.h> +#include <setjmp.h> +#include "cmockery.h" + +#include "postgres.h" +#include "utils/memutils.h" + +#include "../instrument.c" + +#define SIZE_OF_IN_PROGRESS_ARRAY (10 * sizeof(DistributedTransactionId)) + +static void +test__GetTmid_Test(void **state) +{ + assert_true(sizeof(pg_time_t) > sizeof(int32)); + /* + * For very large PgStartTime values, result from timestamptz_to_time_t either overflows or equals to -1, + * and should not match the value casted to int32 from gp_gettmid_helper() + */ + TimestampTz delta = 0x4000000000000000 >> 14; + for (TimestampTz time = 0x7FFFFFFFFFFFFFFF - delta; time > 0x3fffffffffffffff; time -= delta) { + PgStartTime = time; + int32 tmid = gp_gettmid_helper(); + pg_time_t res = timestamptz_to_time_t(PgStartTime); + assert_false((int64)tmid - res == 0); + } + /* + * For smaller PgStartTime values, the result from timestamptz_to_time_t casted to int32 + * should match the result from gp_gettmid_helper() + */ + delta /= 4; + for (TimestampTz time = 0x3ffffffffffff - delta; time >= 0; time -= delta) { + PgStartTime = time; + int32 tmid = gp_gettmid_helper(); + pg_time_t res = timestamptz_to_time_t(PgStartTime); + assert_true((int64)tmid - res == 0); + } + + /* gp_gettmid_helper should return -1 for negative PgStartTime */ + PgStartTime = -100; + int32 tmid = gp_gettmid_helper(); + assert_true(tmid == -1); + + /* gp_gettmid_helper should return -1 for very large PgStartTime value */ + PgStartTime = 0x7FFFFFFFFFFFFFFF - delta; + tmid = gp_gettmid_helper(); + assert_true(tmid == -1); +} + +int +main(int argc, char* argv[]) +{ + cmockery_parse_arguments(argc, argv); + + const UnitTest tests[] = + { + unit_test(test__GetTmid_Test) + }; + + MemoryContextInit(); + + return run_tests(tests); +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
