From a2315fdc677675b420913ca4f81116e165d52397 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Wed, 16 Aug 2023 16:18:28 +0300 Subject: [PATCH] PG16 compatibility - new options to vacuum and analyze (#7114) PG16 compatibility - part 10 Check out part 1 https://github.com/citusdata/citus/commit/42d956888d5be65153ccf24cb039027ecd7c0192 part 2 https://github.com/citusdata/citus/commit/0d503dd5ac5547ca71cd0147e53236d8d8a22fce part 3 https://github.com/citusdata/citus/commit/907d72e60d4043924f52200b24d281fe7b79f75f part 4 https://github.com/citusdata/citus/commit/7c6b4ce1035491ff5a31a9d15bb8b28f3c0dd5b3 part 5 https://github.com/citusdata/citus/commit/6056cb2c2931ae33b42f009872385af518cf2f8b part 6 https://github.com/citusdata/citus/commit/b36c431abbe3f70ba18de5610570adfa9d72d56d part 7 https://github.com/citusdata/citus/commit/ee3153fe5062821e8b83fb7bf62a67a20bbb5098 part 8 https://github.com/citusdata/citus/commit/2c50b5f7ff5e0c08c92a93b2b3292c403826a32a part 9 https://github.com/citusdata/citus/commit/b2291374b4e894b00c1b166ac424072ff8c29bee This commit is in the series of PG16 compatibility commits. It: - Adds buffer_usage_limit to vacuum and analyze - Adds process_main, skip_database_stats, only_database_stats to vacuum Important Note: adding these options is actually required for check-vanilla tests to succeed. However, in concept, this PR belongs to "PG16 new features", rather than "PG16 regression tests sanity" Relevant PG commits: https://github.com/postgres/postgres/commit/1cbbee03385763b066ae3961fc61f2cd01a0d0d7 1cbbee03385763b066ae3961fc61f2cd01a0d0d7 https://github.com/postgres/postgres/commit/4211fbd8413b26e0abedbe4338aa7cda2cd469b4 4211fbd8413b26e0abedbe4338aa7cda2cd469b4 https://github.com/postgres/postgres/commit/a46a7011b27188af526047a111969f257aaf4db8 a46a7011b27188af526047a111969f257aaf4db8 More PG16 compatibility commits are coming soon ... --- src/backend/distributed/commands/vacuum.c | 67 ++++++++++++++++++++++ src/test/regress/expected/pg16.out | 70 +++++++++++++++++++++++ src/test/regress/expected/pg16_0.out | 9 +++ src/test/regress/multi_schedule | 1 + src/test/regress/sql/pg16.sql | 50 ++++++++++++++++ 5 files changed, 197 insertions(+) create mode 100644 src/test/regress/expected/pg16.out create mode 100644 src/test/regress/expected/pg16_0.out create mode 100644 src/test/regress/sql/pg16.sql diff --git a/src/backend/distributed/commands/vacuum.c b/src/backend/distributed/commands/vacuum.c index 4b787adb0..ee03aeae1 100644 --- a/src/backend/distributed/commands/vacuum.c +++ b/src/backend/distributed/commands/vacuum.c @@ -42,6 +42,9 @@ typedef struct CitusVacuumParams VacOptValue truncate; VacOptValue index_cleanup; int nworkers; +#if PG_VERSION_NUM >= PG_VERSION_16 + int ring_size; +#endif } CitusVacuumParams; /* Local functions forward declarations for processing distributed table commands */ @@ -318,10 +321,19 @@ DeparseVacuumStmtPrefix(CitusVacuumParams vacuumParams) } /* if no flags remain, exit early */ +#if PG_VERSION_NUM >= PG_VERSION_16 + if (vacuumFlags & VACOPT_PROCESS_TOAST && + vacuumFlags & VACOPT_PROCESS_MAIN) + { + /* process toast and process main are true by default */ + if (((vacuumFlags & ~VACOPT_PROCESS_TOAST) & ~VACOPT_PROCESS_MAIN) == 0 && + vacuumParams.ring_size == -1 && +#else if (vacuumFlags & VACOPT_PROCESS_TOAST) { /* process toast is true by default */ if ((vacuumFlags & ~VACOPT_PROCESS_TOAST) == 0 && +#endif vacuumParams.truncate == VACOPTVALUE_UNSPECIFIED && vacuumParams.index_cleanup == VACOPTVALUE_UNSPECIFIED && vacuumParams.nworkers == VACUUM_PARALLEL_NOTSET @@ -369,6 +381,28 @@ DeparseVacuumStmtPrefix(CitusVacuumParams vacuumParams) appendStringInfoString(vacuumPrefix, "PROCESS_TOAST FALSE,"); } +#if PG_VERSION_NUM >= PG_VERSION_16 + if (!(vacuumFlags & VACOPT_PROCESS_MAIN)) + { + appendStringInfoString(vacuumPrefix, "PROCESS_MAIN FALSE,"); + } + + if (vacuumFlags & VACOPT_SKIP_DATABASE_STATS) + { + appendStringInfoString(vacuumPrefix, "SKIP_DATABASE_STATS,"); + } + + if (vacuumFlags & VACOPT_ONLY_DATABASE_STATS) + { + appendStringInfoString(vacuumPrefix, "ONLY_DATABASE_STATS,"); + } + + if (vacuumParams.ring_size != -1) + { + appendStringInfo(vacuumPrefix, "BUFFER_USAGE_LIMIT %d,", vacuumParams.ring_size); + } +#endif + if (vacuumParams.truncate != VACOPTVALUE_UNSPECIFIED) { appendStringInfoString(vacuumPrefix, @@ -505,6 +539,13 @@ VacuumStmtParams(VacuumStmt *vacstmt) bool disable_page_skipping = false; bool process_toast = true; +#if PG_VERSION_NUM >= PG_VERSION_16 + bool process_main = true; + bool skip_database_stats = false; + bool only_database_stats = false; + params.ring_size = -1; +#endif + /* Set default value */ params.index_cleanup = VACOPTVALUE_UNSPECIFIED; params.truncate = VACOPTVALUE_UNSPECIFIED; @@ -523,6 +564,13 @@ VacuumStmtParams(VacuumStmt *vacstmt) { skip_locked = defGetBoolean(opt); } +#if PG_VERSION_NUM >= PG_VERSION_16 + else if (strcmp(opt->defname, "buffer_usage_limit") == 0) + { + char *vac_buffer_size = defGetString(opt); + parse_int(vac_buffer_size, ¶ms.ring_size, GUC_UNIT_KB, NULL); + } +#endif else if (!vacstmt->is_vacuumcmd) { ereport(ERROR, @@ -547,6 +595,20 @@ VacuumStmtParams(VacuumStmt *vacstmt) { disable_page_skipping = defGetBoolean(opt); } +#if PG_VERSION_NUM >= PG_VERSION_16 + else if (strcmp(opt->defname, "process_main") == 0) + { + process_main = defGetBoolean(opt); + } + else if (strcmp(opt->defname, "skip_database_stats") == 0) + { + skip_database_stats = defGetBoolean(opt); + } + else if (strcmp(opt->defname, "only_database_stats") == 0) + { + only_database_stats = defGetBoolean(opt); + } +#endif else if (strcmp(opt->defname, "process_toast") == 0) { process_toast = defGetBoolean(opt); @@ -617,6 +679,11 @@ VacuumStmtParams(VacuumStmt *vacstmt) (analyze ? VACOPT_ANALYZE : 0) | (freeze ? VACOPT_FREEZE : 0) | (full ? VACOPT_FULL : 0) | +#if PG_VERSION_NUM >= PG_VERSION_16 + (process_main ? VACOPT_PROCESS_MAIN : 0) | + (skip_database_stats ? VACOPT_SKIP_DATABASE_STATS : 0) | + (only_database_stats ? VACOPT_ONLY_DATABASE_STATS : 0) | +#endif (process_toast ? VACOPT_PROCESS_TOAST : 0) | (disable_page_skipping ? VACOPT_DISABLE_PAGE_SKIPPING : 0); return params; diff --git a/src/test/regress/expected/pg16.out b/src/test/regress/expected/pg16.out new file mode 100644 index 000000000..37580d8a7 --- /dev/null +++ b/src/test/regress/expected/pg16.out @@ -0,0 +1,70 @@ +-- +-- PG16 +-- +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 16 AS server_version_ge_16 +\gset +\if :server_version_ge_16 +\else +\q +\endif +CREATE SCHEMA pg16; +SET search_path TO pg16; +SET citus.next_shard_id TO 950000; +SET citus.shard_count TO 1; +SET citus.shard_replication_factor TO 1; +-- test the new vacuum and analyze options +-- Relevant PG commits: +-- https://github.com/postgres/postgres/commit/1cbbee03385763b066ae3961fc61f2cd01a0d0d7 +-- https://github.com/postgres/postgres/commit/4211fbd8413b26e0abedbe4338aa7cda2cd469b4 +-- https://github.com/postgres/postgres/commit/a46a7011b27188af526047a111969f257aaf4db8 +CREATE TABLE t1 (a int); +SELECT create_distributed_table('t1','a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +SET citus.log_remote_commands TO ON; +VACUUM (PROCESS_MAIN FALSE) t1; +NOTICE: issuing VACUUM (PROCESS_MAIN FALSE) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (PROCESS_MAIN FALSE, PROCESS_TOAST FALSE) t1; +NOTICE: issuing VACUUM (PROCESS_TOAST FALSE,PROCESS_MAIN FALSE) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (PROCESS_MAIN TRUE) t1; +NOTICE: issuing VACUUM pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (PROCESS_MAIN FALSE, FULL) t1; +NOTICE: issuing VACUUM (FULL,PROCESS_MAIN FALSE) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (SKIP_DATABASE_STATS) t1; +NOTICE: issuing VACUUM (SKIP_DATABASE_STATS) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (ONLY_DATABASE_STATS) t1; +ERROR: ONLY_DATABASE_STATS cannot be specified with a list of tables +VACUUM (BUFFER_USAGE_LIMIT '512 kB') t1; +NOTICE: issuing VACUUM (BUFFER_USAGE_LIMIT 512) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (BUFFER_USAGE_LIMIT 0) t1; +NOTICE: issuing VACUUM (BUFFER_USAGE_LIMIT 0) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +VACUUM (BUFFER_USAGE_LIMIT 16777220) t1; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT -1) t1; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +VACUUM (BUFFER_USAGE_LIMIT 'test') t1; +ERROR: BUFFER_USAGE_LIMIT option must be 0 or between 128 kB and 16777216 kB +ANALYZE (BUFFER_USAGE_LIMIT '512 kB') t1; +NOTICE: issuing ANALYZE (BUFFER_USAGE_LIMIT 512) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +ANALYZE (BUFFER_USAGE_LIMIT 0) t1; +NOTICE: issuing ANALYZE (BUFFER_USAGE_LIMIT 0) pg16.t1_950000 +DETAIL: on server postgres@localhost:xxxxx connectionId: xxxxxxx +SET citus.log_remote_commands TO OFF; +-- only verifying it works and not printing log +-- remote commands because it can be flaky +VACUUM (ONLY_DATABASE_STATS); +\set VERBOSITY terse +SET client_min_messages TO ERROR; +DROP SCHEMA pg16 CASCADE; diff --git a/src/test/regress/expected/pg16_0.out b/src/test/regress/expected/pg16_0.out new file mode 100644 index 000000000..730c916ca --- /dev/null +++ b/src/test/regress/expected/pg16_0.out @@ -0,0 +1,9 @@ +-- +-- PG16 +-- +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 16 AS server_version_ge_16 +\gset +\if :server_version_ge_16 +\else +\q diff --git a/src/test/regress/multi_schedule b/src/test/regress/multi_schedule index 19040f1e0..65a272566 100644 --- a/src/test/regress/multi_schedule +++ b/src/test/regress/multi_schedule @@ -65,6 +65,7 @@ test: pg13 pg12 test: pg14 test: pg15 test: pg15_jsonpath detect_conn_close +test: pg16 test: drop_column_partitioned_table test: tableam diff --git a/src/test/regress/sql/pg16.sql b/src/test/regress/sql/pg16.sql new file mode 100644 index 000000000..29638ac1c --- /dev/null +++ b/src/test/regress/sql/pg16.sql @@ -0,0 +1,50 @@ +-- +-- PG16 +-- +SHOW server_version \gset +SELECT substring(:'server_version', '\d+')::int >= 16 AS server_version_ge_16 +\gset +\if :server_version_ge_16 +\else +\q +\endif + +CREATE SCHEMA pg16; +SET search_path TO pg16; +SET citus.next_shard_id TO 950000; +SET citus.shard_count TO 1; +SET citus.shard_replication_factor TO 1; + +-- test the new vacuum and analyze options +-- Relevant PG commits: +-- https://github.com/postgres/postgres/commit/1cbbee03385763b066ae3961fc61f2cd01a0d0d7 +-- https://github.com/postgres/postgres/commit/4211fbd8413b26e0abedbe4338aa7cda2cd469b4 +-- https://github.com/postgres/postgres/commit/a46a7011b27188af526047a111969f257aaf4db8 + +CREATE TABLE t1 (a int); +SELECT create_distributed_table('t1','a'); +SET citus.log_remote_commands TO ON; + +VACUUM (PROCESS_MAIN FALSE) t1; +VACUUM (PROCESS_MAIN FALSE, PROCESS_TOAST FALSE) t1; +VACUUM (PROCESS_MAIN TRUE) t1; +VACUUM (PROCESS_MAIN FALSE, FULL) t1; +VACUUM (SKIP_DATABASE_STATS) t1; +VACUUM (ONLY_DATABASE_STATS) t1; +VACUUM (BUFFER_USAGE_LIMIT '512 kB') t1; +VACUUM (BUFFER_USAGE_LIMIT 0) t1; +VACUUM (BUFFER_USAGE_LIMIT 16777220) t1; +VACUUM (BUFFER_USAGE_LIMIT -1) t1; +VACUUM (BUFFER_USAGE_LIMIT 'test') t1; +ANALYZE (BUFFER_USAGE_LIMIT '512 kB') t1; +ANALYZE (BUFFER_USAGE_LIMIT 0) t1; + +SET citus.log_remote_commands TO OFF; + +-- only verifying it works and not printing log +-- remote commands because it can be flaky +VACUUM (ONLY_DATABASE_STATS); + +\set VERBOSITY terse +SET client_min_messages TO ERROR; +DROP SCHEMA pg16 CASCADE;