From 371f094b681fd879a4230f24c0a53c48ab3ead59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCrkan=20=C4=B0ndibay?= Date: Mon, 21 Aug 2023 17:29:44 +0300 Subject: [PATCH] Removes pg_send_cancellation (#7135) DESCRIPTION: Removes pg_send_cancellation and all references --- Makefile | 16 +- .../distributed/test/pg_send_cancellation.c | 70 ----- src/bin/pg_send_cancellation/.gitignore | 1 - src/bin/pg_send_cancellation/Makefile | 24 -- src/bin/pg_send_cancellation/README.md | 47 ---- .../pg_send_cancellation.c | 261 ------------------ .../regress/enterprise_isolation_schedule | 1 - .../isolation_pg_send_cancellation.out | 42 --- .../spec/isolation_pg_send_cancellation.spec | 65 ----- 9 files changed, 4 insertions(+), 523 deletions(-) delete mode 100644 src/backend/distributed/test/pg_send_cancellation.c delete mode 100644 src/bin/pg_send_cancellation/.gitignore delete mode 100644 src/bin/pg_send_cancellation/Makefile delete mode 100644 src/bin/pg_send_cancellation/README.md delete mode 100644 src/bin/pg_send_cancellation/pg_send_cancellation.c delete mode 100644 src/test/regress/expected/isolation_pg_send_cancellation.out delete mode 100644 src/test/regress/spec/isolation_pg_send_cancellation.spec diff --git a/Makefile b/Makefile index 098b7c207..e42d0ffd3 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ endif include Makefile.global -all: extension pg_send_cancellation +all: extension # build columnar only @@ -40,22 +40,14 @@ clean-full: install-downgrades: $(MAKE) -C src/backend/distributed/ install-downgrades -install-all: install-headers install-pg_send_cancellation +install-all: install-headers $(MAKE) -C src/backend/columnar/ install-all $(MAKE) -C src/backend/distributed/ install-all -# build citus_send_cancellation binary -pg_send_cancellation: - $(MAKE) -C src/bin/pg_send_cancellation/ all -install-pg_send_cancellation: pg_send_cancellation - $(MAKE) -C src/bin/pg_send_cancellation/ install -clean-pg_send_cancellation: - $(MAKE) -C src/bin/pg_send_cancellation/ clean -.PHONY: pg_send_cancellation install-pg_send_cancellation clean-pg_send_cancellation # Add to generic targets -install: install-extension install-headers install-pg_send_cancellation -clean: clean-extension clean-pg_send_cancellation +install: install-extension install-headers +clean: clean-extension # apply or check style reindent: diff --git a/src/backend/distributed/test/pg_send_cancellation.c b/src/backend/distributed/test/pg_send_cancellation.c deleted file mode 100644 index 576d915a6..000000000 --- a/src/backend/distributed/test/pg_send_cancellation.c +++ /dev/null @@ -1,70 +0,0 @@ -/*------------------------------------------------------------------------- - * - * pg_send_cancellation.c - * - * This file contains functions to test setting pg_send_cancellation. - * - * Copyright (c) Citus Data, Inc. - * - *------------------------------------------------------------------------- - */ - -#include "postgres.h" -#include "miscadmin.h" -#include "fmgr.h" -#include "port.h" - -#include "postmaster/postmaster.h" - - -#define PG_SEND_CANCELLATION_VERSION \ - "pg_send_cancellation (PostgreSQL) " PG_VERSION "\n" - - -/* exports for SQL callable functions */ -PG_FUNCTION_INFO_V1(get_cancellation_key); -PG_FUNCTION_INFO_V1(run_pg_send_cancellation); - - -/* - * get_cancellation_key returns the cancellation key of the current process - * as an integer. - */ -Datum -get_cancellation_key(PG_FUNCTION_ARGS) -{ - PG_RETURN_INT32(MyCancelKey); -} - - -/* - * run_pg_send_cancellation runs the pg_send_cancellation program with - * the specified arguments - */ -Datum -run_pg_send_cancellation(PG_FUNCTION_ARGS) -{ - int pid = PG_GETARG_INT32(0); - int cancelKey = PG_GETARG_INT32(1); - - char sendCancellationPath[MAXPGPATH]; - char command[1024]; - - /* Locate executable backend before we change working directory */ - if (find_other_exec(my_exec_path, "pg_send_cancellation", - PG_SEND_CANCELLATION_VERSION, - sendCancellationPath) < 0) - { - ereport(ERROR, (errmsg("could not locate pg_send_cancellation"))); - } - - pg_snprintf(command, sizeof(command), "%s %d %d %s %d", - sendCancellationPath, pid, cancelKey, "localhost", PostPortNumber); - - if (system(command) != 0) - { - ereport(ERROR, (errmsg("failed to run command: %s", command))); - } - - PG_RETURN_VOID(); -} diff --git a/src/bin/pg_send_cancellation/.gitignore b/src/bin/pg_send_cancellation/.gitignore deleted file mode 100644 index 8088a2e98..000000000 --- a/src/bin/pg_send_cancellation/.gitignore +++ /dev/null @@ -1 +0,0 @@ -pg_send_cancellation diff --git a/src/bin/pg_send_cancellation/Makefile b/src/bin/pg_send_cancellation/Makefile deleted file mode 100644 index 4515c5019..000000000 --- a/src/bin/pg_send_cancellation/Makefile +++ /dev/null @@ -1,24 +0,0 @@ -citus_top_builddir = ../../.. - -PROGRAM = pg_send_cancellation -PGFILEDESC = "pg_send_cancellation sends a custom cancellation message" -OBJS = $(citus_abs_srcdir)/src/bin/pg_send_cancellation/pg_send_cancellation.o -PG_CPPFLAGS = -I$(libpq_srcdir) -PG_LIBS_INTERNAL = $(libpq_pgport) -PG_LDFLAGS += $(LDFLAGS) - -include $(citus_top_builddir)/Makefile.global - -# We reuse all the Citus flags (incl. security flags), but we are building a program not a shared library -# We sometimes build Citus with a newer version of gcc than Postgres was built -# with and this breaks LTO (link-time optimization). Even if disabling it can -# have some perf impact this is ok because pg_send_cancellation is only used -# for tests anyway. -override CFLAGS := $(filter-out -shared, $(CFLAGS)) -fno-lto - -# Filter out unneeded dependencies -override LIBS := $(filter-out -lz -lreadline -ledit -ltermcap -lncurses -lcurses -lpam, $(LIBS)) - -clean: clean-pg_send_cancellation -clean-pg_send_cancellation: - rm -f $(PROGRAM) $(OBJS) diff --git a/src/bin/pg_send_cancellation/README.md b/src/bin/pg_send_cancellation/README.md deleted file mode 100644 index c83316419..000000000 --- a/src/bin/pg_send_cancellation/README.md +++ /dev/null @@ -1,47 +0,0 @@ -# pg_send_cancellation - -pg_send_cancellation is a program for manually sending a cancellation -to a Postgres endpoint. It is effectively a command-line version of -PQcancel in libpq, but it can use any PID or cancellation key. - -We use pg_send_cancellation primarily to propagate cancellations between pgbouncers -behind a load balancer. Since the cancellation protocol involves -opening a new connection, the new connection may go to a different -node that does not recognize the cancellation key. To handle that -scenario, we modified pgbouncer to pass unrecognized cancellation -keys to a shell command. - -Users can configure the cancellation_command, which will be run with: -``` - -``` - -Note that pgbouncer does not use actual PIDs. Instead, it generates PID and cancellation key together a random 8-byte number. This makes the chance of collisions exceedingly small. - -By providing pg_send_cancellation as part of Citus, we can use a shell script that pgbouncer invokes to propagate the cancellation to all *other* worker nodes in the same cluster, for example: - -```bash -#!/bin/sh -remote_ip=$1 -remote_port=$2 -pid=$3 -cancel_key=$4 - -postgres_path=/usr/pgsql-14/bin -pgbouncer_port=6432 - -nodes_query="select nodename from pg_dist_node where groupid > 0 and groupid not in (select groupid from pg_dist_local_group) and nodecluster = current_setting('citus.cluster_name')" - -# Get hostnames of other worker nodes in the cluster, and send cancellation to their pgbouncers -$postgres_path/psql -c "$nodes_query" -tAX | xargs -n 1 sh -c "$postgres_path/pg_send_cancellation $pid $cancel_key \$0 $pgbouncer_port" -``` - -One thing we need to be careful about is that the cancellations do not get forwarded -back-and-forth. This is handled in pgbouncer by setting the last bit of all generated -cancellation keys (sent to clients) to 1, and setting the last bit of all forwarded bits to 0. -That way, when a pgbouncer receives a cancellation key with the last bit set to 0, -it knows it is from another pgbouncer and should not forward further, and should set -the last bit to 1 when comparing to stored cancellation keys. - -Another thing we need to be careful about is that the integers should be encoded -as big endian on the wire. diff --git a/src/bin/pg_send_cancellation/pg_send_cancellation.c b/src/bin/pg_send_cancellation/pg_send_cancellation.c deleted file mode 100644 index 0ab2be95a..000000000 --- a/src/bin/pg_send_cancellation/pg_send_cancellation.c +++ /dev/null @@ -1,261 +0,0 @@ -/* - * pg_send_cancellation is a program for manually sending a cancellation - * to a Postgres endpoint. It is effectively a command-line version of - * PQcancel in libpq, but it can use any PID or cancellation key. - * - * Portions Copyright (c) Citus Data, Inc. - * - * For the internal_cancel function: - * - * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * Permission to use, copy, modify, and distribute this software and its - * documentation for any purpose, without fee, and without a written agreement - * is hereby granted, provided that the above copyright notice and this - * paragraph and the following two paragraphs appear in all copies. - * - * IN NO EVENT SHALL THE UNIVERSITY OF CALIFORNIA BE LIABLE TO ANY PARTY FOR - * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING - * LOST PROFITS, ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS - * DOCUMENTATION, EVEN IF THE UNIVERSITY OF CALIFORNIA HAS BEEN ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - * - * THE UNIVERSITY OF CALIFORNIA SPECIFICALLY DISCLAIMS ANY WARRANTIES, - * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY - * AND FITNESS FOR A PARTICULAR PURPOSE. THE SOFTWARE PROVIDED HEREUNDER IS - * ON AN "AS IS" BASIS, AND THE UNIVERSITY OF CALIFORNIA HAS NO OBLIGATIONS TO - * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. - * - */ -#include "postgres_fe.h" - -#include -#include -#include -#include -#include - -#include "common/ip.h" -#include "common/link-canary.h" -#include "common/scram-common.h" -#include "common/string.h" -#include "libpq-fe.h" -#include "libpq-int.h" -#include "mb/pg_wchar.h" -#include "port/pg_bswap.h" - - -#define ERROR_BUFFER_SIZE 256 - - -static int internal_cancel(SockAddr *raddr, int be_pid, int be_key, - char *errbuf, int errbufsize); - - -/* - * main entry point into the pg_send_cancellation program. - */ -int -main(int argc, char *argv[]) -{ - if (argc == 2 && strcmp(argv[1], "-V") == 0) - { - pg_fprintf(stdout, "pg_send_cancellation (PostgreSQL) " PG_VERSION "\n"); - return 0; - } - - if (argc < 4 || argc > 5) - { - char *program = argv[0]; - pg_fprintf(stderr, "%s requires 4 arguments\n\n", program); - pg_fprintf(stderr, "Usage: %s [port]\n", program); - return 1; - } - - char *pidString = argv[1]; - char *cancelKeyString = argv[2]; - char *host = argv[3]; - char *portString = "5432"; - - if (argc >= 5) - { - portString = argv[4]; - } - - /* parse the PID and cancellation key */ - int pid = strtol(pidString, NULL, 10); - int cancelAuthCode = strtol(cancelKeyString, NULL, 10); - - char errorBuffer[ERROR_BUFFER_SIZE] = { 0 }; - - struct addrinfo *ipAddressList; - struct addrinfo hint; - int ipAddressListFamily = AF_UNSPEC; - SockAddr socketAddress; - - memset(&hint, 0, sizeof(hint)); - hint.ai_socktype = SOCK_STREAM; - hint.ai_family = ipAddressListFamily; - - /* resolve the hostname to an IP */ - int ret = pg_getaddrinfo_all(host, portString, &hint, &ipAddressList); - if (ret || !ipAddressList) - { - pg_fprintf(stderr, "could not translate host name \"%s\" to address: %s\n", - host, gai_strerror(ret)); - return 1; - } - - if (ipAddressList->ai_addrlen > sizeof(socketAddress.addr)) - { - pg_fprintf(stderr, "invalid address length"); - return 1; - } - - /* - * Explanation of IGNORE-BANNED: - * This is a common pattern when using getaddrinfo. The system guarantees - * that ai_addrlen < sizeof(socketAddress.addr). Out of an abundance of - * caution. We also check it above. - */ - memcpy(&socketAddress.addr, ipAddressList->ai_addr, ipAddressList->ai_addrlen); /* IGNORE-BANNED */ - socketAddress.salen = ipAddressList->ai_addrlen; - - /* send the cancellation */ - bool cancelSucceeded = internal_cancel(&socketAddress, pid, cancelAuthCode, - errorBuffer, sizeof(errorBuffer)); - if (!cancelSucceeded) - { - pg_fprintf(stderr, "sending cancellation to %s:%s failed: %s", - host, portString, errorBuffer); - return 1; - } - - pg_freeaddrinfo_all(ipAddressListFamily, ipAddressList); - - return 0; -} - - -/* *INDENT-OFF* */ - -/* - * internal_cancel is copied from fe-connect.c - * - * The return value is true if the cancel request was successfully - * dispatched, false if not (in which case an error message is available). - * Note: successful dispatch is no guarantee that there will be any effect at - * the backend. The application must read the operation result as usual. - * - * CAUTION: we want this routine to be safely callable from a signal handler - * (for example, an application might want to call it in a SIGINT handler). - * This means we cannot use any C library routine that might be non-reentrant. - * malloc/free are often non-reentrant, and anything that might call them is - * just as dangerous. We avoid sprintf here for that reason. Building up - * error messages with strcpy/strcat is tedious but should be quite safe. - * We also save/restore errno in case the signal handler support doesn't. - * - * internal_cancel() is an internal helper function to make code-sharing - * between the two versions of the cancel function possible. - */ -static int -internal_cancel(SockAddr *raddr, int be_pid, int be_key, - char *errbuf, int errbufsize) -{ - int save_errno = SOCK_ERRNO; - pgsocket tmpsock = PGINVALID_SOCKET; - char sebuf[PG_STRERROR_R_BUFLEN]; - int maxlen; - struct - { - uint32 packetlen; - CancelRequestPacket cp; - } crp; - - /* - * We need to open a temporary connection to the postmaster. Do this with - * only kernel calls. - */ - if ((tmpsock = socket(raddr->addr.ss_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET) - { - strlcpy(errbuf, "PQcancel() -- socket() failed: ", errbufsize); - goto cancel_errReturn; - } -retry3: - if (connect(tmpsock, (struct sockaddr *) &raddr->addr, raddr->salen) < 0) - { - if (SOCK_ERRNO == EINTR) - /* Interrupted system call - we'll just try again */ - goto retry3; - strlcpy(errbuf, "PQcancel() -- connect() failed: ", errbufsize); - goto cancel_errReturn; - } - - /* - * We needn't set nonblocking I/O or NODELAY options here. - */ - - /* Create and send the cancel request packet. */ - - crp.packetlen = pg_hton32((uint32) sizeof(crp)); - crp.cp.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); - crp.cp.backendPID = pg_hton32(be_pid); - crp.cp.cancelAuthCode = pg_hton32(be_key); - -retry4: - if (send(tmpsock, (char *) &crp, sizeof(crp), 0) != (int) sizeof(crp)) - { - if (SOCK_ERRNO == EINTR) - /* Interrupted system call - we'll just try again */ - goto retry4; - strlcpy(errbuf, "PQcancel() -- send() failed: ", errbufsize); - goto cancel_errReturn; - } - - /* - * Wait for the postmaster to close the connection, which indicates that - * it's processed the request. Without this delay, we might issue another - * command only to find that our cancel zaps that command instead of the - * one we thought we were canceling. Note we don't actually expect this - * read to obtain any data, we are just waiting for EOF to be signaled. - */ -retry5: - if (recv(tmpsock, (char *) &crp, 1, 0) < 0) - { - if (SOCK_ERRNO == EINTR) - /* Interrupted system call - we'll just try again */ - goto retry5; - /* we ignore other error conditions */ - } - - /* All done */ - closesocket(tmpsock); - SOCK_ERRNO_SET(save_errno); - return true; - -cancel_errReturn: - - /* - * Make sure we don't overflow the error buffer. Leave space for the \n at - * the end, and for the terminating zero. - */ - maxlen = errbufsize - strlen(errbuf) - 2; - if (maxlen >= 0) - { - /* - * Explanation of IGNORE-BANNED: - * This is well-tested libpq code that we would like to preserve in its - * original form. The appropriate length calculation is done above. - */ - strncat(errbuf, SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)), /* IGNORE-BANNED */ - maxlen); - strcat(errbuf, "\n"); /* IGNORE-BANNED */ - } - if (tmpsock != PGINVALID_SOCKET) - closesocket(tmpsock); - SOCK_ERRNO_SET(save_errno); - return false; -} - -/* *INDENT-ON* */ diff --git a/src/test/regress/enterprise_isolation_schedule b/src/test/regress/enterprise_isolation_schedule index 3e6655f88..689a7db75 100644 --- a/src/test/regress/enterprise_isolation_schedule +++ b/src/test/regress/enterprise_isolation_schedule @@ -10,7 +10,6 @@ test: isolation_move_placement_vs_modification test: isolation_move_placement_vs_modification_fk test: isolation_tenant_isolation_with_fkey_to_reference test: isolation_ref2ref_foreign_keys_enterprise -test: isolation_pg_send_cancellation test: isolation_shard_move_vs_start_metadata_sync test: isolation_tenant_isolation test: isolation_tenant_isolation_nonblocking diff --git a/src/test/regress/expected/isolation_pg_send_cancellation.out b/src/test/regress/expected/isolation_pg_send_cancellation.out deleted file mode 100644 index 4b1475352..000000000 --- a/src/test/regress/expected/isolation_pg_send_cancellation.out +++ /dev/null @@ -1,42 +0,0 @@ -Parsed test spec with 2 sessions - -starting permutation: s1-register s2-lock s1-lock s2-wrong-cancel-1 s2-wrong-cancel-2 s2-cancel -step s1-register: - INSERT INTO cancel_table VALUES (pg_backend_pid(), get_cancellation_key()); - -step s2-lock: - BEGIN; - LOCK TABLE cancel_table IN ACCESS EXCLUSIVE MODE; - -step s1-lock: - BEGIN; - LOCK TABLE cancel_table IN ACCESS EXCLUSIVE MODE; - END; - -step s2-wrong-cancel-1: - SELECT run_pg_send_cancellation(pid + 1, cancel_key) FROM cancel_table; - -run_pg_send_cancellation ---------------------------------------------------------------------- - -(1 row) - -step s2-wrong-cancel-2: - SELECT run_pg_send_cancellation(pid, cancel_key + 1) FROM cancel_table; - -run_pg_send_cancellation ---------------------------------------------------------------------- - -(1 row) - -step s2-cancel: - SELECT run_pg_send_cancellation(pid, cancel_key) FROM cancel_table; - END; - -run_pg_send_cancellation ---------------------------------------------------------------------- - -(1 row) - -step s1-lock: <... completed> -ERROR: canceling statement due to user request diff --git a/src/test/regress/spec/isolation_pg_send_cancellation.spec b/src/test/regress/spec/isolation_pg_send_cancellation.spec deleted file mode 100644 index 46c6a0539..000000000 --- a/src/test/regress/spec/isolation_pg_send_cancellation.spec +++ /dev/null @@ -1,65 +0,0 @@ -setup -{ - CREATE FUNCTION run_pg_send_cancellation(int,int) - RETURNS void - AS 'citus' - LANGUAGE C STRICT; - - CREATE FUNCTION get_cancellation_key() - RETURNS int - AS 'citus' - LANGUAGE C STRICT; - - CREATE TABLE cancel_table (pid int, cancel_key int); -} - -teardown -{ - DROP TABLE IF EXISTS cancel_table; -} - -session "s1" - -/* store the PID and cancellation key of session 1 */ -step "s1-register" -{ - INSERT INTO cancel_table VALUES (pg_backend_pid(), get_cancellation_key()); -} - -/* lock the table from session 1, will block and get cancelled */ -step "s1-lock" -{ - BEGIN; - LOCK TABLE cancel_table IN ACCESS EXCLUSIVE MODE; - END; -} - -session "s2" - -/* lock the table from session 2 to block session 1 */ -step "s2-lock" -{ - BEGIN; - LOCK TABLE cancel_table IN ACCESS EXCLUSIVE MODE; -} - -/* PID mismatch */ -step "s2-wrong-cancel-1" -{ - SELECT run_pg_send_cancellation(pid + 1, cancel_key) FROM cancel_table; -} - -/* cancellation key mismatch */ -step "s2-wrong-cancel-2" -{ - SELECT run_pg_send_cancellation(pid, cancel_key + 1) FROM cancel_table; -} - -/* cancel the LOCK statement in session 1 */ -step "s2-cancel" -{ - SELECT run_pg_send_cancellation(pid, cancel_key) FROM cancel_table; - END; -} - -permutation "s1-register" "s2-lock" "s1-lock" "s2-wrong-cancel-1" "s2-wrong-cancel-2" "s2-cancel"