From 2a263fe69a707d16ef24378f7650742386b0968f Mon Sep 17 00:00:00 2001 From: Hanefi Onaldi Date: Wed, 17 Jul 2024 15:21:51 +0300 Subject: [PATCH 1/4] Add changelog entries for 12.1.5 (#7648) --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6184e61ce..78d1d2a7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +### citus v12.1.5 (July 17, 2024) ### + +* Adds support for MERGE commands with single shard distributed target tables + (#7643) + +* Fixes an error with MERGE commands when insert value does not have source + distribution column (#7627) + ### citus v12.1.4 (May 28, 2024) ### * Adds null check for node in HasRangeTableRef (#7604) From 9e1852eac7374de87898ea21a3862584609f77e8 Mon Sep 17 00:00:00 2001 From: eaydingol <60466783+eaydingol@users.noreply.github.com> Date: Wed, 31 Jul 2024 14:53:38 +0300 Subject: [PATCH 2/4] Check if the limit is null (#7665) DESCRIPTION: Add a check to see if the given limit is null. Fixes a bug by checking if the limit given in the query is null when the actual limit is computed with respect to the given offset. Prior to this change, null is interpreted as 0 during the limit calculation when both limit and offset are given. Fixes #7663 --- .../planner/multi_logical_optimizer.c | 29 +++++-- .../regress/expected/multi_limit_clause.out | 80 +++++++++++++++++++ src/test/regress/sql/multi_limit_clause.sql | 19 +++++ 3 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/backend/distributed/planner/multi_logical_optimizer.c b/src/backend/distributed/planner/multi_logical_optimizer.c index 76e38237a..371ba54e6 100644 --- a/src/backend/distributed/planner/multi_logical_optimizer.c +++ b/src/backend/distributed/planner/multi_logical_optimizer.c @@ -4753,22 +4753,35 @@ WorkerLimitCount(Node *limitCount, Node *limitOffset, OrderByLimitReference if (workerLimitNode != NULL && limitOffset != NULL) { Const *workerLimitConst = (Const *) workerLimitNode; - Const *workerOffsetConst = (Const *) limitOffset; - int64 workerLimitCount = DatumGetInt64(workerLimitConst->constvalue); - int64 workerOffsetCount = DatumGetInt64(workerOffsetConst->constvalue); - workerLimitCount = workerLimitCount + workerOffsetCount; - workerLimitNode = (Node *) MakeIntegerConstInt64(workerLimitCount); + /* Only update the worker limit if the const is not null.*/ + if (!workerLimitConst->constisnull) + { + Const *workerOffsetConst = (Const *) limitOffset; + int64 workerLimitCount = DatumGetInt64(workerLimitConst->constvalue); + + /* If the offset is null, it defaults to 0 when cast to int64. */ + int64 workerOffsetCount = DatumGetInt64(workerOffsetConst->constvalue); + workerLimitCount = workerLimitCount + workerOffsetCount; + workerLimitNode = (Node *) MakeIntegerConstInt64(workerLimitCount); + } } /* display debug message on limit push down */ if (workerLimitNode != NULL) { Const *workerLimitConst = (Const *) workerLimitNode; - int64 workerLimitCount = DatumGetInt64(workerLimitConst->constvalue); + if (!workerLimitConst->constisnull) + { + int64 workerLimitCount = DatumGetInt64(workerLimitConst->constvalue); - ereport(DEBUG1, (errmsg("push down of limit count: " INT64_FORMAT, - workerLimitCount))); + ereport(DEBUG1, (errmsg("push down of limit count: " INT64_FORMAT, + workerLimitCount))); + } + else + { + ereport(DEBUG1, (errmsg("push down of limit count: ALL"))); + } } return workerLimitNode; diff --git a/src/test/regress/expected/multi_limit_clause.out b/src/test/regress/expected/multi_limit_clause.out index 65304b777..83cd05837 100644 --- a/src/test/regress/expected/multi_limit_clause.out +++ b/src/test/regress/expected/multi_limit_clause.out @@ -521,6 +521,86 @@ SELECT 1 | 1 (1 row) +-- check if we can correctly push the limit when it is null +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey LIMIT null; +DEBUG: push down of limit count: ALL + l_orderkey +--------------------------------------------------------------------- + 1 + 1 + 1 + 1 + 1 + 1 + 2 +(7 rows) + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET 1 LIMIT null; +DEBUG: push down of limit count: ALL + l_orderkey +--------------------------------------------------------------------- + 1 + 1 + 1 + 1 + 1 + 2 +(6 rows) + +SELECT count(*) FROM lineitem LIMIT null; +DEBUG: push down of limit count: ALL + count +--------------------------------------------------------------------- + 12000 +(1 row) + +SELECT count(*) FROM lineitem OFFSET 0 LIMIT null; +DEBUG: push down of limit count: ALL + count +--------------------------------------------------------------------- + 12000 +(1 row) + +-- check if we push the right limit when both offset and limit are given +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET 1 LIMIT 3; +DEBUG: push down of limit count: 4 + l_orderkey +--------------------------------------------------------------------- + 1 + 1 + 1 +(3 rows) + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET null LIMIT 1; +DEBUG: push down of limit count: 1 + l_orderkey +--------------------------------------------------------------------- + 1 +(1 row) + +-- check if we can correctly push the limit when it is all +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 2 LIMIT all; +DEBUG: push down of limit count: ALL + l_orderkey +--------------------------------------------------------------------- + 1 + 1 + 1 + 1 + 1 + 1 +(6 rows) + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 2 OFFSET 2 LIMIT all; +DEBUG: push down of limit count: ALL + l_orderkey +--------------------------------------------------------------------- + 1 + 1 + 1 + 1 +(4 rows) + SET client_min_messages TO NOTICE; -- non constants should not push down CREATE OR REPLACE FUNCTION my_limit() diff --git a/src/test/regress/sql/multi_limit_clause.sql b/src/test/regress/sql/multi_limit_clause.sql index 8d14bbbc8..5e3b3e3de 100644 --- a/src/test/regress/sql/multi_limit_clause.sql +++ b/src/test/regress/sql/multi_limit_clause.sql @@ -222,6 +222,25 @@ SELECT ORDER BY 2 DESC, 1 LIMIT 5; +-- check if we can correctly push the limit when it is null +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey LIMIT null; + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET 1 LIMIT null; + +SELECT count(*) FROM lineitem LIMIT null; + +SELECT count(*) FROM lineitem OFFSET 0 LIMIT null; + +-- check if we push the right limit when both offset and limit are given +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET 1 LIMIT 3; + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 3 ORDER BY l_orderkey OFFSET null LIMIT 1; + +-- check if we can correctly push the limit when it is all +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 2 LIMIT all; + +SELECT l_orderkey FROM lineitem WHERE l_orderkey < 2 OFFSET 2 LIMIT all; + SET client_min_messages TO NOTICE; -- non constants should not push down From 68d28ecdc0b1b4c289ae12e830362ee41a7265a2 Mon Sep 17 00:00:00 2001 From: Mehmet YILMAZ Date: Fri, 23 Aug 2024 12:16:18 +0300 Subject: [PATCH 3/4] Add Debugging Instructions to Devcontainer Setup in CONTRIBUTING.md (#7673) **Description:** This PR adds a section to CONTRIBUTING.md that explains how to set up debugging in the devcontainer using VS Code. **Changes:** - **New Debugging Section**: Clear instructions on starting the debugger, selecting the appropriate PostgreSQL process, and setting breakpoints for easier troubleshooting. **Purpose:** - **Improved Contributor Workflow**: Enables contributors to debug the Citus extension within the devcontainer, enhancing productivity and making it easier to resolve issues. --------- Co-authored-by: Mehmet YILMAZ --- .devcontainer/.psqlrc | 2 +- CONTRIBUTING.md | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/.devcontainer/.psqlrc b/.devcontainer/.psqlrc index 7642a9714..07ea06cdd 100644 --- a/.devcontainer/.psqlrc +++ b/.devcontainer/.psqlrc @@ -3,5 +3,5 @@ \pset border 2 \setenv PAGER 'pspg --no-mouse -bX --no-commandbar --no-topbar' \set HISTSIZE 100000 -\set PROMPT1 '\n%[%033[1m%]%M %n@%/:%>-%p%R%[%033[0m%]%# ' +\set PROMPT1 '\n%[%033[1m%]%M %n@%/:%> (PID: %p)%R%[%033[0m%]%# ' \set PROMPT2 ' ' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e1900642d..70cc486e7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,6 +35,28 @@ To get citus installed from source we run `make install -s` in the first termina With the Citus cluster running you can connect to the coordinator in the first terminal via `psql -p9700`. Because the coordinator is the most common entrypoint the `PGPORT` environment is set accordingly, so a simple `psql` will connect directly to the coordinator. +### Debugging in the VS code + +1. Start Debugging: Press F5 in VS Code to start debugging. When prompted, you'll need to attach the debugger to the appropriate PostgreSQL process. + +2. Identify the Process: If you're running a psql command, take note of the PID that appears in your psql prompt. For example: +``` +[local] citus@citus:9700 (PID: 5436)=# +``` +This PID (5436 in this case) indicates the process that you should attach the debugger to. +If you are uncertain about which process to attach, you can list all running PostgreSQL processes using the following command: +``` +ps aux | grep postgres +``` + +Look for the process associated with the PID you noted. For example: +``` +citus 5436 0.0 0.0 0 0 ? S 14:00 0:00 postgres: citus citus +``` +4. Attach the Debugger: Once you've identified the correct PID, select that process when prompted in VS Code to attach the debugger. You should now be able to debug the PostgreSQL session tied to the psql command. + +5. Set Breakpoints and Debug: With the debugger attached, you can set breakpoints within the code. This allows you to step through the code execution, inspect variables, and fully debug the PostgreSQL instance running in your container. + ### Getting and building [PostgreSQL documentation](https://www.postgresql.org/support/versioning/) has a From 477571569178ca8f48321bc396f1db07b6f2244f Mon Sep 17 00:00:00 2001 From: Mehmet YILMAZ Date: Mon, 9 Sep 2024 17:09:56 +0300 Subject: [PATCH 4/4] Fix race condition in citus_set_coordinator_host when adding multiple coordinator nodes concurrently (#7682) When multiple sessions concurrently attempt to add the same coordinator node using `citus_set_coordinator_host`, there is a potential race condition. Both sessions may pass the initial metadata check (`isCoordinatorInMetadata`), but only one will succeed in adding the node. The other session will fail with an assertion error (`Assert(!nodeAlreadyExists)`), causing the server to crash. Even though the `AddNodeMetadata` function takes an exclusive lock, it appears that the lock is not preventing the race condition before the initial metadata check. - **Issue**: The current logic allows concurrent sessions to pass the check for existing coordinators, leading to an attempt to insert duplicate nodes, which triggers the assertion failure. - **Impact**: This race condition leads to crashes during operations that involve concurrent coordinator additions, as seen in https://github.com/citusdata/citus/issues/7646. **Test Plan:** - Isolation Test Limitation: An isolation test was added to simulate concurrent additions of the same coordinator node, but due to the behavior of PostgreSQL locking mechanisms, the test does not trigger the edge case. The lock applied within the function serializes the operations, preventing the race condition from occurring in the isolation test environment. While the edge case is difficult to reproduce in an isolation test, the fix addresses the core issue by ensuring concurrency control through proper locking. - Existing Tests: All existing tests related to node metadata and coordinator management have been run to ensure that no regressions were introduced. **After the Fix:** - Concurrent attempts to add the same coordinator node will be serialized. One session will succeed in adding the node, while the others will skip the operation without crashing the server. Co-authored-by: Mehmet YILMAZ --- citus-tools | 1 + src/backend/distributed/metadata/node_metadata.c | 3 +++ 2 files changed, 4 insertions(+) create mode 160000 citus-tools diff --git a/citus-tools b/citus-tools new file mode 160000 index 000000000..3376bd684 --- /dev/null +++ b/citus-tools @@ -0,0 +1 @@ +Subproject commit 3376bd6845f0614908ed304f5033bd644c82d3bf diff --git a/src/backend/distributed/metadata/node_metadata.c b/src/backend/distributed/metadata/node_metadata.c index d93b133ea..d92205943 100644 --- a/src/backend/distributed/metadata/node_metadata.c +++ b/src/backend/distributed/metadata/node_metadata.c @@ -217,6 +217,9 @@ citus_set_coordinator_host(PG_FUNCTION_ARGS) EnsureTransactionalMetadataSyncMode(); } + /* prevent concurrent modification */ + LockRelationOid(DistNodeRelationId(), RowExclusiveLock); + bool isCoordinatorInMetadata = false; WorkerNode *coordinatorNode = PrimaryNodeForGroup(COORDINATOR_GROUP_ID, &isCoordinatorInMetadata);