From 6fc7544f18eac351941422ef6a980582d9f7df02 Mon Sep 17 00:00:00 2001 From: Nitish Upreti Date: Tue, 12 Jul 2022 15:42:20 -0700 Subject: [PATCH] Remove split_mode and use shard_transfer_mode instead' --- .../citus_split_shard_by_split_points.c | 26 ++++++++++------- .../sql/downgrades/citus--11.1-1--11.0-3.sql | 3 +- .../11.1-1.sql | 16 +++-------- .../latest.sql | 16 +++-------- .../citus_split_shard_by_split_points.out | 6 ++-- ...s_split_shard_by_split_points_negative.out | 19 +++++++++++++ .../isolation_blocking_shard_split.out | 28 +++++++++---------- ...ing_shard_split_with_fkey_to_reference.out | 10 +++---- src/test/regress/expected/multi_extension.out | 3 +- .../expected/upgrade_list_citus_objects.out | 3 +- .../spec/isolation_blocking_shard_split.spec | 4 +-- ...ng_shard_split_with_fkey_to_reference.spec | 2 +- .../sql/citus_split_shard_by_split_points.sql | 6 ++-- ...s_split_shard_by_split_points_negative.sql | 19 +++++++++++++ 14 files changed, 93 insertions(+), 68 deletions(-) diff --git a/src/backend/distributed/operations/citus_split_shard_by_split_points.c b/src/backend/distributed/operations/citus_split_shard_by_split_points.c index da9c12497..d40245f48 100644 --- a/src/backend/distributed/operations/citus_split_shard_by_split_points.c +++ b/src/backend/distributed/operations/citus_split_shard_by_split_points.c @@ -30,12 +30,12 @@ PG_FUNCTION_INFO_V1(citus_split_shard_by_split_points); static SplitMode LookupSplitMode(Oid shardSplitModeOid); /* - * citus_split_shard_by_split_points(shard_id bigint, split_points text[], node_ids integer[], split_mode citus.split_mode) + * citus_split_shard_by_split_points(shard_id bigint, split_points text[], node_ids integer[], shard_transfer_mode citus.shard_transfer_mode) * Split source shard into multiple shards using the given split points. * 'shard_id' is the id of source shard to split. * 'split_points' is an array that represents the split points. * 'node_ids' is an array that represents the placement node ids of the new shards. - * 'split_mode citus.split_mode' is the mode of split. + * 'shard_transfer_mode citus.shard_transfer_mode' is the transfer mode for split. */ Datum citus_split_shard_by_split_points(PG_FUNCTION_ARGS) @@ -51,8 +51,8 @@ citus_split_shard_by_split_points(PG_FUNCTION_ARGS) ArrayType *nodeIdsArrayObject = PG_GETARG_ARRAYTYPE_P(2); List *nodeIdsForPlacementList = IntegerArrayTypeToList(nodeIdsArrayObject); - Oid shardSplitModeOid = PG_GETARG_OID(3); - SplitMode shardSplitMode = LookupSplitMode(shardSplitModeOid); + Oid shardTransferModeOid = PG_GETARG_OID(3); + SplitMode shardSplitMode = LookupSplitMode(shardTransferModeOid); SplitShard( shardSplitMode, @@ -66,25 +66,31 @@ citus_split_shard_by_split_points(PG_FUNCTION_ARGS) /* - * LookupSplitMode maps the oids of citus.shard_split_mode enum - * values to an enum. + * LookupSplitMode maps the oids of citus.shard_transfer_mode to SplitMode enum. */ SplitMode -LookupSplitMode(Oid shardSplitModeOid) +LookupSplitMode(Oid shardTransferModeOid) { SplitMode shardSplitMode = BLOCKING_SPLIT; - Datum enumLabelDatum = DirectFunctionCall1(enum_out, shardSplitModeOid); + Datum enumLabelDatum = DirectFunctionCall1(enum_out, shardTransferModeOid); char *enumLabel = DatumGetCString(enumLabelDatum); /* Extend with other modes as we support them */ - if (strncmp(enumLabel, "blocking", NAMEDATALEN) == 0) + if (strncmp(enumLabel, "block_writes", NAMEDATALEN) == 0) { shardSplitMode = BLOCKING_SPLIT; } + else if (strncmp(enumLabel, "auto", NAMEDATALEN) == 0 || + strncmp(enumLabel, "force_logical", NAMEDATALEN) == 0) + { + ereport(ERROR, (errmsg("Shard Tranfer mode: '%s' is not supported. Please use 'block_writes' instead.", + enumLabel))); + } else { - ereport(ERROR, (errmsg("Invalid split mode: %s. Expected split mode is blocking.", + // We will not get here as postgres will validate the enum value. + ereport(ERROR, (errmsg("Invalid shard tranfer mode: '%s'. Expected split mode is 'block_writes'.", enumLabel))); } diff --git a/src/backend/distributed/sql/downgrades/citus--11.1-1--11.0-3.sql b/src/backend/distributed/sql/downgrades/citus--11.1-1--11.0-3.sql index f7dc5ca07..90f54c2c5 100644 --- a/src/backend/distributed/sql/downgrades/citus--11.1-1--11.0-3.sql +++ b/src/backend/distributed/sql/downgrades/citus--11.1-1--11.0-3.sql @@ -50,11 +50,10 @@ DROP FUNCTION pg_catalog.citus_split_shard_by_split_points( shard_id bigint, split_points text[], node_ids integer[], - split_mode citus.split_mode); + shard_transfer_mode citus.shard_transfer_mode); DROP FUNCTION pg_catalog.worker_split_copy( source_shard_id bigint, splitCopyInfos citus.split_copy_info[]); -DROP TYPE citus.split_mode; DROP TYPE citus.split_copy_info; #include "../../../columnar/sql/downgrades/columnar--11.1-1--11.0-3.sql" diff --git a/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/11.1-1.sql b/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/11.1-1.sql index 559769260..36624c40e 100644 --- a/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/11.1-1.sql +++ b/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/11.1-1.sql @@ -1,22 +1,14 @@ -DROP TYPE IF EXISTS citus.split_mode; - --- Three modes to be implemented: blocking, non_blocking and auto. --- Currently, the default / only supported mode is blocking. -CREATE TYPE citus.split_mode AS ENUM ( - 'blocking' -); - CREATE OR REPLACE FUNCTION pg_catalog.citus_split_shard_by_split_points( shard_id bigint, split_points text[], -- A 'nodeId' is a uint32 in CITUS [1, 4294967296] but postgres does not have unsigned type support. -- Use integer (consistent with other previously defined UDFs that take nodeId as integer) as for all practical purposes it is big enough. node_ids integer[], - -- Three modes to be implemented: blocking, non_blocking and auto. - -- Currently, the default / only supported mode is blocking. - split_mode citus.split_mode default 'blocking') + -- Three modes to be implemented: block_writes, force_logical and auto. + -- Currently, the default / only supported mode is block_writes. + shard_transfer_mode citus.shard_transfer_mode default 'block_writes') RETURNS void LANGUAGE C STRICT AS 'MODULE_PATHNAME', $$citus_split_shard_by_split_points$$; -COMMENT ON FUNCTION pg_catalog.citus_split_shard_by_split_points(shard_id bigint, split_points text[], nodeIds integer[], citus.split_mode) +COMMENT ON FUNCTION pg_catalog.citus_split_shard_by_split_points(shard_id bigint, split_points text[], nodeIds integer[], citus.shard_transfer_mode) IS 'split a shard using split mode.'; diff --git a/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/latest.sql b/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/latest.sql index 559769260..36624c40e 100644 --- a/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/latest.sql +++ b/src/backend/distributed/sql/udfs/citus_split_shard_by_split_points/latest.sql @@ -1,22 +1,14 @@ -DROP TYPE IF EXISTS citus.split_mode; - --- Three modes to be implemented: blocking, non_blocking and auto. --- Currently, the default / only supported mode is blocking. -CREATE TYPE citus.split_mode AS ENUM ( - 'blocking' -); - CREATE OR REPLACE FUNCTION pg_catalog.citus_split_shard_by_split_points( shard_id bigint, split_points text[], -- A 'nodeId' is a uint32 in CITUS [1, 4294967296] but postgres does not have unsigned type support. -- Use integer (consistent with other previously defined UDFs that take nodeId as integer) as for all practical purposes it is big enough. node_ids integer[], - -- Three modes to be implemented: blocking, non_blocking and auto. - -- Currently, the default / only supported mode is blocking. - split_mode citus.split_mode default 'blocking') + -- Three modes to be implemented: block_writes, force_logical and auto. + -- Currently, the default / only supported mode is block_writes. + shard_transfer_mode citus.shard_transfer_mode default 'block_writes') RETURNS void LANGUAGE C STRICT AS 'MODULE_PATHNAME', $$citus_split_shard_by_split_points$$; -COMMENT ON FUNCTION pg_catalog.citus_split_shard_by_split_points(shard_id bigint, split_points text[], nodeIds integer[], citus.split_mode) +COMMENT ON FUNCTION pg_catalog.citus_split_shard_by_split_points(shard_id bigint, split_points text[], nodeIds integer[], citus.shard_transfer_mode) IS 'split a shard using split mode.'; diff --git a/src/test/regress/expected/citus_split_shard_by_split_points.out b/src/test/regress/expected/citus_split_shard_by_split_points.out index 2d008dc24..85bae93e4 100644 --- a/src/test/regress/expected/citus_split_shard_by_split_points.out +++ b/src/test/regress/expected/citus_split_shard_by_split_points.out @@ -213,7 +213,7 @@ SELECT pg_catalog.citus_split_shard_by_split_points( 8981000, ARRAY['-1073741824'], ARRAY[:worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -224,7 +224,7 @@ SELECT pg_catalog.citus_split_shard_by_split_points( 8981001, ARRAY['536870911', '1610612735'], ARRAY[:worker_1_node, :worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -386,7 +386,7 @@ SELECT pg_catalog.citus_split_shard_by_split_points( 8981007, ARRAY['-2100000000'], ARRAY[:worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- diff --git a/src/test/regress/expected/citus_split_shard_by_split_points_negative.out b/src/test/regress/expected/citus_split_shard_by_split_points_negative.out index 148e3a142..5986fa74b 100644 --- a/src/test/regress/expected/citus_split_shard_by_split_points_negative.out +++ b/src/test/regress/expected/citus_split_shard_by_split_points_negative.out @@ -32,6 +32,25 @@ SELECT create_distributed_table('table_to_split','id'); SELECT nodeid AS worker_1_node FROM pg_dist_node WHERE nodeport=:worker_1_port \gset SELECT nodeid AS worker_2_node FROM pg_dist_node WHERE nodeport=:worker_2_port \gset +-- UDF fails for any other shard_transfer_mode other than block_writes. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'auto'); +ERROR: Shard Tranfer mode: 'auto' is not supported. Please use 'block_writes' instead. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'force_logical'); +ERROR: Shard Tranfer mode: 'force_logical' is not supported. Please use 'block_writes' instead. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'gibberish'); +ERROR: invalid input value for enum citus.shard_transfer_mode: "gibberish" -- UDF fails for range partitioned tables. SELECT citus_split_shard_by_split_points( 60761300, diff --git a/src/test/regress/expected/isolation_blocking_shard_split.out b/src/test/regress/expected/isolation_blocking_shard_split.out index ff3c250fd..02a23174e 100644 --- a/src/test/regress/expected/isolation_blocking_shard_split.out +++ b/src/test/regress/expected/isolation_blocking_shard_split.out @@ -42,7 +42,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -126,7 +126,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -200,7 +200,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -280,7 +280,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -359,7 +359,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -439,7 +439,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -509,7 +509,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -585,7 +585,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -657,7 +657,7 @@ step s1-blocking-shard-split: 1500001, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -669,7 +669,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); step s1-commit: COMMIT; @@ -732,7 +732,7 @@ step s1-blocking-shard-split: 1500001, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -744,7 +744,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); step s1-commit: COMMIT; @@ -812,7 +812,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -895,7 +895,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- diff --git a/src/test/regress/expected/isolation_blocking_shard_split_with_fkey_to_reference.out b/src/test/regress/expected/isolation_blocking_shard_split_with_fkey_to_reference.out index 410b9c2a0..9a6ed53eb 100644 --- a/src/test/regress/expected/isolation_blocking_shard_split_with_fkey_to_reference.out +++ b/src/test/regress/expected/isolation_blocking_shard_split_with_fkey_to_reference.out @@ -20,7 +20,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -80,7 +80,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -140,7 +140,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -200,7 +200,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- @@ -260,7 +260,7 @@ step s2-blocking-shard-split: 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); citus_split_shard_by_split_points --------------------------------------------------------------------- diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 19004dd8f..ce728db90 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -1095,7 +1095,7 @@ SELECT * FROM multi_extension.print_extension_changes(); table columnar.chunk_group | table columnar.options | table columnar.stripe | - | function citus_split_shard_by_split_points(bigint,text[],integer[],citus.split_mode) void + | function citus_split_shard_by_split_points(bigint,text[],integer[],citus.shard_transfer_mode) void | function columnar.get_storage_id(regclass) bigint | function columnar_internal.columnar_handler(internal) table_am_handler | function worker_split_copy(bigint,citus.split_copy_info[]) void @@ -1106,7 +1106,6 @@ SELECT * FROM multi_extension.print_extension_changes(); | table columnar_internal.options | table columnar_internal.stripe | type citus.split_copy_info - | type citus.split_mode | view columnar.chunk | view columnar.chunk_group | view columnar.options diff --git a/src/test/regress/expected/upgrade_list_citus_objects.out b/src/test/regress/expected/upgrade_list_citus_objects.out index 6063ac3ec..614d95a9b 100644 --- a/src/test/regress/expected/upgrade_list_citus_objects.out +++ b/src/test/regress/expected/upgrade_list_citus_objects.out @@ -113,7 +113,7 @@ ORDER BY 1; function citus_shard_indexes_on_worker() function citus_shard_sizes() function citus_shards_on_worker() - function citus_split_shard_by_split_points(bigint,text[],integer[],citus.split_mode) + function citus_split_shard_by_split_points(bigint,text[],integer[],citus.shard_transfer_mode) function citus_stat_activity() function citus_stat_statements() function citus_stat_statements_reset() @@ -272,7 +272,6 @@ ORDER BY 1; type citus.distribution_type type citus.shard_transfer_mode type citus.split_copy_info - type citus.split_mode type citus_copy_format type noderole view citus_dist_stat_activity diff --git a/src/test/regress/spec/isolation_blocking_shard_split.spec b/src/test/regress/spec/isolation_blocking_shard_split.spec index a06824886..ddac66f5b 100644 --- a/src/test/regress/spec/isolation_blocking_shard_split.spec +++ b/src/test/regress/spec/isolation_blocking_shard_split.spec @@ -70,7 +70,7 @@ step "s1-blocking-shard-split" 1500001, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); } step "s1-commit" @@ -91,7 +91,7 @@ step "s2-blocking-shard-split" 1500002, ARRAY['1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); } step "s2-commit" diff --git a/src/test/regress/spec/isolation_blocking_shard_split_with_fkey_to_reference.spec b/src/test/regress/spec/isolation_blocking_shard_split_with_fkey_to_reference.spec index 243d8ef05..49b56c4a5 100644 --- a/src/test/regress/spec/isolation_blocking_shard_split_with_fkey_to_reference.spec +++ b/src/test/regress/spec/isolation_blocking_shard_split_with_fkey_to_reference.spec @@ -67,7 +67,7 @@ step "s2-blocking-shard-split" 1500002, ARRAY['-1073741824'], ARRAY[1, 2], - 'blocking'); + 'block_writes'); } step "s2-add-fkey" diff --git a/src/test/regress/sql/citus_split_shard_by_split_points.sql b/src/test/regress/sql/citus_split_shard_by_split_points.sql index 7448a488a..ec553c0da 100644 --- a/src/test/regress/sql/citus_split_shard_by_split_points.sql +++ b/src/test/regress/sql/citus_split_shard_by_split_points.sql @@ -132,14 +132,14 @@ SELECT pg_catalog.citus_split_shard_by_split_points( 8981000, ARRAY['-1073741824'], ARRAY[:worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); -- Perform 3 way split SELECT pg_catalog.citus_split_shard_by_split_points( 8981001, ARRAY['536870911', '1610612735'], ARRAY[:worker_1_node, :worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); -- END : Split two shards : One with move and One without move. -- BEGIN : Move a shard post split. @@ -209,7 +209,7 @@ SELECT pg_catalog.citus_split_shard_by_split_points( 8981007, ARRAY['-2100000000'], ARRAY[:worker_1_node, :worker_2_node], - 'blocking'); + 'block_writes'); SET search_path TO "citus_split_test_schema"; SELECT shard.shardid, logicalrelid, shardminvalue, shardmaxvalue, nodename, nodeport diff --git a/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql b/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql index 7f310c055..e730a8c28 100644 --- a/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql +++ b/src/test/regress/sql/citus_split_shard_by_split_points_negative.sql @@ -23,6 +23,25 @@ SELECT create_distributed_table('table_to_split','id'); SELECT nodeid AS worker_1_node FROM pg_dist_node WHERE nodeport=:worker_1_port \gset SELECT nodeid AS worker_2_node FROM pg_dist_node WHERE nodeport=:worker_2_port \gset +-- UDF fails for any other shard_transfer_mode other than block_writes. +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'auto'); + +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'force_logical'); + +SELECT citus_split_shard_by_split_points( + 49761302, + ARRAY['50'], + ARRAY[101, 201], + 'gibberish'); + -- UDF fails for range partitioned tables. SELECT citus_split_shard_by_split_points( 60761300,