inplace040-waitfuncs-v3.patch

text/plain

Filename: inplace040-waitfuncs-v3.patch
Type: text/plain
Part: 3
Message: Re: race condition in pg_class

Patch

Same data as JSON: GET /api/v1/attachments/:id/patch the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes. API reference →
Format: unified
Series: patch v3
File+
src/backend/utils/adt/lockfuncs.c 0 79
src/backend/utils/adt/Makefile 1 0
src/backend/utils/adt/meson.build 1 0
src/backend/utils/adt/waitfuncs.c 96 0
Author:     Noah Misch <noah@leadboat.com>
Commit:     Noah Misch <noah@leadboat.com>

    Create waitfuncs.c for pg_isolation_test_session_is_blocked().
    
    The next commit makes the function inspect an additional non-lock
    contention source, so it no longer fits in lockfuncs.c.
    
    Reviewed by FIXME.
    
    Discussion: https://postgr.es/m/20240512232923.aa.nmisch@google.com

diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 610ccf2..edb09d4 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -116,6 +116,7 @@ OBJS = \
 	varchar.o \
 	varlena.o \
 	version.o \
+	waitfuncs.o \
 	windowfuncs.o \
 	xid.o \
 	xid8funcs.o \
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index 13009cc..e790f85 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -13,7 +13,6 @@
 #include "postgres.h"
 
 #include "access/htup_details.h"
-#include "catalog/pg_type.h"
 #include "funcapi.h"
 #include "miscadmin.h"
 #include "storage/predicate_internals.h"
@@ -602,84 +601,6 @@ pg_safe_snapshot_blocking_pids(PG_FUNCTION_ARGS)
 
 
 /*
- * pg_isolation_test_session_is_blocked - support function for isolationtester
- *
- * Check if specified PID is blocked by any of the PIDs listed in the second
- * argument.  Currently, this looks for blocking caused by waiting for
- * heavyweight locks or safe snapshots.  We ignore blockage caused by PIDs
- * not directly under the isolationtester's control, eg autovacuum.
- *
- * This is an undocumented function intended for use by the isolation tester,
- * and may change in future releases as required for testing purposes.
- */
-Datum
-pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
-{
-	int			blocked_pid = PG_GETARG_INT32(0);
-	ArrayType  *interesting_pids_a = PG_GETARG_ARRAYTYPE_P(1);
-	ArrayType  *blocking_pids_a;
-	int32	   *interesting_pids;
-	int32	   *blocking_pids;
-	int			num_interesting_pids;
-	int			num_blocking_pids;
-	int			dummy;
-	int			i,
-				j;
-
-	/* Validate the passed-in array */
-	Assert(ARR_ELEMTYPE(interesting_pids_a) == INT4OID);
-	if (array_contains_nulls(interesting_pids_a))
-		elog(ERROR, "array must not contain nulls");
-	interesting_pids = (int32 *) ARR_DATA_PTR(interesting_pids_a);
-	num_interesting_pids = ArrayGetNItems(ARR_NDIM(interesting_pids_a),
-										  ARR_DIMS(interesting_pids_a));
-
-	/*
-	 * Get the PIDs of all sessions blocking the given session's attempt to
-	 * acquire heavyweight locks.
-	 */
-	blocking_pids_a =
-		DatumGetArrayTypeP(DirectFunctionCall1(pg_blocking_pids, blocked_pid));
-
-	Assert(ARR_ELEMTYPE(blocking_pids_a) == INT4OID);
-	Assert(!array_contains_nulls(blocking_pids_a));
-	blocking_pids = (int32 *) ARR_DATA_PTR(blocking_pids_a);
-	num_blocking_pids = ArrayGetNItems(ARR_NDIM(blocking_pids_a),
-									   ARR_DIMS(blocking_pids_a));
-
-	/*
-	 * Check if any of these are in the list of interesting PIDs, that being
-	 * the sessions that the isolation tester is running.  We don't use
-	 * "arrayoverlaps" here, because it would lead to cache lookups and one of
-	 * our goals is to run quickly with debug_discard_caches > 0.  We expect
-	 * blocking_pids to be usually empty and otherwise a very small number in
-	 * isolation tester cases, so make that the outer loop of a naive search
-	 * for a match.
-	 */
-	for (i = 0; i < num_blocking_pids; i++)
-		for (j = 0; j < num_interesting_pids; j++)
-		{
-			if (blocking_pids[i] == interesting_pids[j])
-				PG_RETURN_BOOL(true);
-		}
-
-	/*
-	 * Check if blocked_pid is waiting for a safe snapshot.  We could in
-	 * theory check the resulting array of blocker PIDs against the
-	 * interesting PIDs list, but since there is no danger of autovacuum
-	 * blocking GetSafeSnapshot there seems to be no point in expending cycles
-	 * on allocating a buffer and searching for overlap; so it's presently
-	 * sufficient for the isolation tester's purposes to use a single element
-	 * buffer and check if the number of safe snapshot blockers is non-zero.
-	 */
-	if (GetSafeSnapshotBlockingPids(blocked_pid, &dummy, 1) > 0)
-		PG_RETURN_BOOL(true);
-
-	PG_RETURN_BOOL(false);
-}
-
-
-/*
  * Functions for manipulating advisory locks
  *
  * We make use of the locktag fields as follows:
diff --git a/src/backend/utils/adt/meson.build b/src/backend/utils/adt/meson.build
index 48dbcf5..8c6fc80 100644
--- a/src/backend/utils/adt/meson.build
+++ b/src/backend/utils/adt/meson.build
@@ -103,6 +103,7 @@ backend_sources += files(
   'varchar.c',
   'varlena.c',
   'version.c',
+  'waitfuncs.c',
   'windowfuncs.c',
   'xid.c',
   'xid8funcs.c',
diff --git a/src/backend/utils/adt/waitfuncs.c b/src/backend/utils/adt/waitfuncs.c
new file mode 100644
index 0000000..d9c92c3
--- /dev/null
+++ b/src/backend/utils/adt/waitfuncs.c
@@ -0,0 +1,96 @@
+/*-------------------------------------------------------------------------
+ *
+ * waitfuncs.c
+ *		Functions for SQL access to syntheses of multiple contention types.
+ *
+ * Copyright (c) 2002-2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *		src/backend/utils/adt/waitfuncs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "catalog/pg_type.h"
+#include "storage/predicate_internals.h"
+#include "utils/array.h"
+#include "utils/builtins.h"
+
+
+/*
+ * pg_isolation_test_session_is_blocked - support function for isolationtester
+ *
+ * Check if specified PID is blocked by any of the PIDs listed in the second
+ * argument.  Currently, this looks for blocking caused by waiting for
+ * heavyweight locks or safe snapshots.  We ignore blockage caused by PIDs
+ * not directly under the isolationtester's control, eg autovacuum.
+ *
+ * This is an undocumented function intended for use by the isolation tester,
+ * and may change in future releases as required for testing purposes.
+ */
+Datum
+pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
+{
+	int			blocked_pid = PG_GETARG_INT32(0);
+	ArrayType  *interesting_pids_a = PG_GETARG_ARRAYTYPE_P(1);
+	ArrayType  *blocking_pids_a;
+	int32	   *interesting_pids;
+	int32	   *blocking_pids;
+	int			num_interesting_pids;
+	int			num_blocking_pids;
+	int			dummy;
+	int			i,
+				j;
+
+	/* Validate the passed-in array */
+	Assert(ARR_ELEMTYPE(interesting_pids_a) == INT4OID);
+	if (array_contains_nulls(interesting_pids_a))
+		elog(ERROR, "array must not contain nulls");
+	interesting_pids = (int32 *) ARR_DATA_PTR(interesting_pids_a);
+	num_interesting_pids = ArrayGetNItems(ARR_NDIM(interesting_pids_a),
+										  ARR_DIMS(interesting_pids_a));
+
+	/*
+	 * Get the PIDs of all sessions blocking the given session's attempt to
+	 * acquire heavyweight locks.
+	 */
+	blocking_pids_a =
+		DatumGetArrayTypeP(DirectFunctionCall1(pg_blocking_pids, blocked_pid));
+
+	Assert(ARR_ELEMTYPE(blocking_pids_a) == INT4OID);
+	Assert(!array_contains_nulls(blocking_pids_a));
+	blocking_pids = (int32 *) ARR_DATA_PTR(blocking_pids_a);
+	num_blocking_pids = ArrayGetNItems(ARR_NDIM(blocking_pids_a),
+									   ARR_DIMS(blocking_pids_a));
+
+	/*
+	 * Check if any of these are in the list of interesting PIDs, that being
+	 * the sessions that the isolation tester is running.  We don't use
+	 * "arrayoverlaps" here, because it would lead to cache lookups and one of
+	 * our goals is to run quickly with debug_discard_caches > 0.  We expect
+	 * blocking_pids to be usually empty and otherwise a very small number in
+	 * isolation tester cases, so make that the outer loop of a naive search
+	 * for a match.
+	 */
+	for (i = 0; i < num_blocking_pids; i++)
+		for (j = 0; j < num_interesting_pids; j++)
+		{
+			if (blocking_pids[i] == interesting_pids[j])
+				PG_RETURN_BOOL(true);
+		}
+
+	/*
+	 * Check if blocked_pid is waiting for a safe snapshot.  We could in
+	 * theory check the resulting array of blocker PIDs against the
+	 * interesting PIDs list, but since there is no danger of autovacuum
+	 * blocking GetSafeSnapshot there seems to be no point in expending cycles
+	 * on allocating a buffer and searching for overlap; so it's presently
+	 * sufficient for the isolation tester's purposes to use a single element
+	 * buffer and check if the number of safe snapshot blockers is non-zero.
+	 */
+	if (GetSafeSnapshotBlockingPids(blocked_pid, &dummy, 1) > 0)
+		PG_RETURN_BOOL(true);
+
+	PG_RETURN_BOOL(false);
+}