pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedInjection points: some tools to wait and wake
46+ messages / 4 participants
[nested] [flat]
Commits from this thread:
2024-03-04 eca2c1ea85 Add PostgreSQL::Test::Cluster::wait_for_event()
2024-03-04 6782709df8 Add regression test for restart points during promotion
2024-03-04 37b369dc67 injection_points: Add wait and wakeup of processes
* Injection points: some tools to wait and wake
@ 2024-02-19 06:01 Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 08:54 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 2 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-19 06:01 UTC (permalink / raw)
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Ashutosh Bapat <ashutosh.bapat@gmail.com>
Hi all,
(Ashutosh in CC as he was involved in the discussion last time.)
I have proposed on the original thread related to injection points to
have more stuff to be able to wait at an arbtrary point and wake at
will the process waiting so as it is possible to control the order of
actions taken in a test:
https://www.postgresql.org/message-id/ZTiV8tn_MIb_H2rE%40paquier.xyz
I didn't do that in the other thread out of time, but here is a patch
set to complete what I wanted, using a condition variable to wait and
wake processes:
- State is in shared memory, using a DSM tracked by the registry and
an integer counter.
- Callback to wait on a condition variable.
- SQL function to update the shared state and broadcast the update to
the condition variable.
- Use a custom wait event to track the wait in pg_stat_activity.
0001 requires no backend changes, only more stuff into the test module
injection_points so that could be backpatched assuming that the
backend is able to support injection points. This could be expanded
into using more variables and/or states, but I don't really see a
point in introducing more without a reason to do so, and I have no
need for more at the moment.
0002 is a polished version of the TAP test that makes use of this
facility, providing coverage for the bug fixed by 7863ee4def65
(reverting this commit causes the test to fail), where a restart point
runs across a promotion request. The trick is to stop the
checkpointer in the middle of a restart point and issue a promotion
in-between.
Thoughts and comments are welcome.
--
Michael
Attachments: download all (3 files, .tar.gz)
0001-injection_points-Add-routines-to-wait-and-wake-proce.patch (text/x-diff, 5.8K) download
From ef27d6eba619f5106915de6b05cbeb5294263c30 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 19 Feb 2024 14:35:55 +0900
Subject: [PATCH 1/2] injection_points: Add routines to wait and wake processes
This commit is made of two parts:
- A new callback that can be attached to a process to make it wait on a
condition variable. The condition checked is registered in shared
memory by the module injection_points.
- A new SQL function to update the shared state and broadcast the update
using a condition variable.
The shared state used by the module is registered using the DSM
registry, and is optional.
---
.../injection_points--1.0.sql | 10 ++
.../injection_points/injection_points.c | 104 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 115 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..20479991f2 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -24,6 +24,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_wake()
+--
+-- Wakes a condition variable waited on in an injection point.
+--
+CREATE FUNCTION injection_points_wake()
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_wake'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_detach()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..3a319b1525 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,18 +18,67 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/condition_variable.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
+#include "storage/dsm_registry.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
#include "utils/wait_event.h"
PG_MODULE_MAGIC;
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* protects accesses to wait_counts */
+ slock_t lock;
+
+ /* Counter advancing when injection_points_wake() is called */
+ int wait_counts;
+
+ /*
+ * Condition variable that can be used in an injection point, checking
+ * upon wait_counts when waiting.
+ */
+ ConditionVariable wait_point;
+} InjectionPointSharedState;
+
+/* Pointer to shared-memory state. */
+static InjectionPointSharedState *inj_state = NULL;
+
+/* Wait event when waiting on condition variable */
+static uint32 injection_wait_event = 0;
+
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
+static void
+injection_point_init_state(void *ptr)
+{
+ InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
+
+ state->wait_counts = 0;
+ SpinLockInit(&state->lock);
+ ConditionVariableInit(&state->wait_point);
+}
+
+static void
+injection_init_shmem(void)
+{
+ bool found;
+
+ if (inj_state != NULL)
+ return;
+
+ inj_state = GetNamedDSMSegment("injection_points",
+ sizeof(InjectionPointSharedState),
+ injection_point_init_state,
+ &found);
+}
+
/* Set of callbacks available to be attached to an injection point. */
void
injection_error(const char *name)
@@ -43,6 +92,38 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+/* Wait on a condition variable, awaken by injection_points_wake() */
+void
+injection_wait(const char *name)
+{
+ int old_wait_counts;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+ if (injection_wait_event == 0)
+ injection_wait_event = WaitEventExtensionNew("injection_wait");
+
+ SpinLockAcquire(&inj_state->lock);
+ old_wait_counts = inj_state->wait_counts;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And sleep.. */
+ ConditionVariablePrepareToSleep(&inj_state->wait_point);
+ for (;;)
+ {
+ int new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts;
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
+ ConditionVariableCancelSleep();
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +139,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
@@ -80,6 +163,27 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for waking a condition variable.
+ */
+PG_FUNCTION_INFO_V1(injection_points_wake);
+Datum
+injection_points_wake(PG_FUNCTION_ARGS)
+{
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /* First bump the wait counter */
+ SpinLockAcquire(&inj_state->lock);
+ inj_state->wait_counts++;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And broadcast the change for the waiters */
+ ConditionVariableBroadcast(&inj_state->wait_point);
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for dropping an injection point.
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d808aad8b0..d7eca00502 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1208,6 +1208,7 @@ InitializeDSMForeignScan_function
InitializeWorkerForeignScan_function
InjectionPointCacheEntry
InjectionPointEntry
+InjectionPointSharedState
InlineCodeBlock
InsertStmt
Instrumentation
--
2.43.0
0002-Add-regression-test-for-restart-points-during-promot.patch (text/x-diff, 8.7K) download
From 67797944f91441e0cde1259409eeffcc1bd62e39 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 19 Feb 2024 14:38:08 +0900
Subject: [PATCH 2/2] Add regression test for restart points during promotion
This test fails when 7863ee4def65 is reverted, checking that a node is
able to properly restart following a crash when a restart point was
finishing across a promotion.
This is an old bug that had no coverage, and injection points make that
cheap to achieve.
---
src/backend/access/transam/xlog.c | 7 +
src/test/recovery/Makefile | 7 +-
src/test/recovery/meson.build | 4 +
.../t/041_invalid_checkpoint_after_promote.pl | 159 ++++++++++++++++++
4 files changed, 176 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 50c347a679..50b045ff08 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@
#include "storage/sync.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/relmapper.h"
@@ -7536,6 +7537,12 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /*
+ * This location needs to be after CheckPointGuts() to ensure that some
+ * work has already happened during this checkpoint.
+ */
+ INJECTION_POINT("CreateRestartPoint");
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 17ee353735..f57baba5e8 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,12 +9,17 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm contrib/pg_stat_statements contrib/test_decoding
+EXTRA_INSTALL=contrib/pg_prewarm \
+ contrib/pg_stat_statements \
+ contrib/test_decoding \
+ src/test/modules/injection_points
subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export enable_injection_points enable_injection_points
+
# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index bf087ac2a9..e4e0e2b4cc 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -6,6 +6,9 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'test_kwargs': {'priority': 40}, # recovery tests are slow, start early
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_stream_rep.pl',
't/002_archiving.pl',
@@ -46,6 +49,7 @@ tests += {
't/038_save_logical_slots_shutdown.pl',
't/039_end_of_wal.pl',
't/040_standby_failover_slots_sync.pl',
+ 't/041_invalid_checkpoint_after_promote.pl',
],
},
}
diff --git a/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
new file mode 100644
index 0000000000..b2e9b526ee
--- /dev/null
+++ b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
@@ -0,0 +1,159 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Time::HiRes qw(usleep);
+use Test::More;
+
+##################################################
+# Test race condition when a restart point is running during a promotion,
+# checking that WAL segments are correctly removed in the restart point
+# while the promotion finishes.
+#
+# This test relies on an injection point that causes the checkpointer to
+# wait in the middle of a restart point on a standby. The checkpointer
+# is awaken to finish its restart point only once the promotion of the
+# standby is completed, and the node should be able to restart properly.
+##################################################
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('master');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', q[
+checkpoint_timeout = 30s
+log_checkpoints = on
+restart_after_crash = on
+]);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+# Setup a standby
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Dummy table for the upcoming tests.
+$node_primary->safe_psql('postgres', 'checkpoint');
+$node_primary->safe_psql('postgres', 'CREATE TABLE prim_tab (a int);');
+
+# Register a injection point on the standby so as the follow-up
+# restart point will wait on it.
+$node_primary->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# Wait until the extension has been created on the standby
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Note that from this point the checkpointer will wait in the middle of
+# a restart point on the standby.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_attach('CreateRestartPoint', 'wait');");
+
+# Execute a restart point on the standby, that we will now be waiting on.
+# This needs to be in the background.
+my $logstart = -s $node_standby->logfile;
+my $psql_session =
+ $node_standby->background_psql('postgres', on_error_stop => 0);
+$psql_session->query_until(
+ qr/starting_checkpoint/, q(
+ \echo starting_checkpoint
+ CHECKPOINT;
+));
+
+# Switch one WAL segment to make the previous restart point remove the
+# segment once the restart point completes.
+$node_primary->safe_psql('postgres', 'INSERT INTO prim_tab VALUES (1);');
+$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();');
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'injection_wait' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
+
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+my $log = slurp_file($node_standby->logfile, $logstart);
+my $checkpoint_start = 0;
+if ($log =~ m/restartpoint starting: immediate wait/)
+{
+ $checkpoint_start = 1;
+}
+is($checkpoint_start, 1, 'restartpoint has started');
+
+# Trigger promotion during the restart point.
+$node_primary->stop;
+$node_standby->promote;
+
+# Update the start position before waking up the checkpointer!
+$logstart = -s $node_standby->logfile;
+
+# Now wake up the checkpointer.
+$node_standby->safe_psql('postgres', "SELECT injection_points_wake();");
+
+# Wait until the previous restart point completes on the newly-promoted
+# standby, checking the logs for that.
+my $checkpoint_complete = 0;
+for (my $i = 0; $i < 3000; $i++)
+{
+ my $log = slurp_file($node_standby->logfile, $logstart);
+ if ($log =~ m/restartpoint complete/)
+ {
+ $checkpoint_complete = 1;
+ last;
+ }
+ usleep(100_000);
+}
+is($checkpoint_complete, 1, 'restart point has completed');
+
+# Kill with SIGKILL, forcing all the backends to restart.
+my $psql_timeout = IPC::Run::timer(3600);
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+ [
+ 'psql', '-XAtq', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+ $node_standby->connstr('postgres')
+ ],
+ '<',
+ \$killme_stdin,
+ '>',
+ \$killme_stdout,
+ '2>',
+ \$killme_stderr,
+ $psql_timeout);
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+$killme->pump until $killme_stdout =~ /[[:digit:]]+[\r\n]$/;
+my $pid = $killme_stdout;
+chomp($pid);
+my $ret = PostgreSQL::Test::Utils::system_log('pg_ctl', 'kill', 'KILL', $pid);
+is($ret, 0, 'killed process with KILL');
+my $stdout;
+my $stderr;
+
+# Wait till server finishes restarting
+$node_standby->poll_query_until('postgres', undef, '');
+
+# After recovery, the server should be able to start.
+($ret, $stdout, $stderr) = $node_standby->psql('postgres', 'select 1');
+is($ret, 0, "psql connect success");
+is($stdout, 1, "psql select 1");
+
+done_testing();
--
2.43.0
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-19 07:51 ` Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
1 sibling, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-19 07:51 UTC (permalink / raw)
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>; +Cc: Ashutosh Bapat <ashutosh.bapat@gmail.com>
On Mon, Feb 19, 2024 at 03:01:40PM +0900, Michael Paquier wrote:
> 0002 is a polished version of the TAP test that makes use of this
> facility, providing coverage for the bug fixed by 7863ee4def65
> (reverting this commit causes the test to fail), where a restart point
> runs across a promotion request. The trick is to stop the
> checkpointer in the middle of a restart point and issue a promotion
> in-between.
The CF bot has been screaming at this one on Windows because the
process started with IPC::Run::start was not properly finished, so
attached is an updated version to bring that back to green.
--
Michael
Attachments: download all (3 files, .tar.gz)
v2-0001-injection_points-Add-routines-to-wait-and-wake-pr.patch (text/x-diff, 5.8K) download
From ef27d6eba619f5106915de6b05cbeb5294263c30 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 19 Feb 2024 14:35:55 +0900
Subject: [PATCH v2 1/2] injection_points: Add routines to wait and wake
processes
This commit is made of two parts:
- A new callback that can be attached to a process to make it wait on a
condition variable. The condition checked is registered in shared
memory by the module injection_points.
- A new SQL function to update the shared state and broadcast the update
using a condition variable.
The shared state used by the module is registered using the DSM
registry, and is optional.
---
.../injection_points--1.0.sql | 10 ++
.../injection_points/injection_points.c | 104 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 115 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..20479991f2 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -24,6 +24,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_wake()
+--
+-- Wakes a condition variable waited on in an injection point.
+--
+CREATE FUNCTION injection_points_wake()
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_wake'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_detach()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..3a319b1525 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,18 +18,67 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/condition_variable.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
+#include "storage/dsm_registry.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
#include "utils/wait_event.h"
PG_MODULE_MAGIC;
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* protects accesses to wait_counts */
+ slock_t lock;
+
+ /* Counter advancing when injection_points_wake() is called */
+ int wait_counts;
+
+ /*
+ * Condition variable that can be used in an injection point, checking
+ * upon wait_counts when waiting.
+ */
+ ConditionVariable wait_point;
+} InjectionPointSharedState;
+
+/* Pointer to shared-memory state. */
+static InjectionPointSharedState *inj_state = NULL;
+
+/* Wait event when waiting on condition variable */
+static uint32 injection_wait_event = 0;
+
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
+static void
+injection_point_init_state(void *ptr)
+{
+ InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
+
+ state->wait_counts = 0;
+ SpinLockInit(&state->lock);
+ ConditionVariableInit(&state->wait_point);
+}
+
+static void
+injection_init_shmem(void)
+{
+ bool found;
+
+ if (inj_state != NULL)
+ return;
+
+ inj_state = GetNamedDSMSegment("injection_points",
+ sizeof(InjectionPointSharedState),
+ injection_point_init_state,
+ &found);
+}
+
/* Set of callbacks available to be attached to an injection point. */
void
injection_error(const char *name)
@@ -43,6 +92,38 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+/* Wait on a condition variable, awaken by injection_points_wake() */
+void
+injection_wait(const char *name)
+{
+ int old_wait_counts;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+ if (injection_wait_event == 0)
+ injection_wait_event = WaitEventExtensionNew("injection_wait");
+
+ SpinLockAcquire(&inj_state->lock);
+ old_wait_counts = inj_state->wait_counts;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And sleep.. */
+ ConditionVariablePrepareToSleep(&inj_state->wait_point);
+ for (;;)
+ {
+ int new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts;
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
+ ConditionVariableCancelSleep();
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +139,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
@@ -80,6 +163,27 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for waking a condition variable.
+ */
+PG_FUNCTION_INFO_V1(injection_points_wake);
+Datum
+injection_points_wake(PG_FUNCTION_ARGS)
+{
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /* First bump the wait counter */
+ SpinLockAcquire(&inj_state->lock);
+ inj_state->wait_counts++;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And broadcast the change for the waiters */
+ ConditionVariableBroadcast(&inj_state->wait_point);
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for dropping an injection point.
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d808aad8b0..d7eca00502 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1208,6 +1208,7 @@ InitializeDSMForeignScan_function
InitializeWorkerForeignScan_function
InjectionPointCacheEntry
InjectionPointEntry
+InjectionPointSharedState
InlineCodeBlock
InsertStmt
Instrumentation
--
2.43.0
v2-0002-Add-regression-test-for-restart-points-during-pro.patch (text/x-diff, 9.1K) download
From c64634c5d1826dab6012d8055b494d807ebc6be7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 19 Feb 2024 16:49:36 +0900
Subject: [PATCH v2 2/2] Add regression test for restart points during
promotion
This test fails when 7863ee4def65 is reverted, checking that a node is
able to properly restart following a crash when a restart point was
finishing across a promotion.
This is an old bug that had no coverage, and injection points make that
cheap to achieve.
---
src/backend/access/transam/xlog.c | 7 +
src/test/recovery/Makefile | 7 +-
src/test/recovery/meson.build | 4 +
.../t/041_invalid_checkpoint_after_promote.pl | 175 ++++++++++++++++++
4 files changed, 192 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 50c347a679..50b045ff08 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@
#include "storage/sync.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/relmapper.h"
@@ -7536,6 +7537,12 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /*
+ * This location needs to be after CheckPointGuts() to ensure that some
+ * work has already happened during this checkpoint.
+ */
+ INJECTION_POINT("CreateRestartPoint");
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 17ee353735..f57baba5e8 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,12 +9,17 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm contrib/pg_stat_statements contrib/test_decoding
+EXTRA_INSTALL=contrib/pg_prewarm \
+ contrib/pg_stat_statements \
+ contrib/test_decoding \
+ src/test/modules/injection_points
subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export enable_injection_points enable_injection_points
+
# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index bf087ac2a9..e4e0e2b4cc 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -6,6 +6,9 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'test_kwargs': {'priority': 40}, # recovery tests are slow, start early
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_stream_rep.pl',
't/002_archiving.pl',
@@ -46,6 +49,7 @@ tests += {
't/038_save_logical_slots_shutdown.pl',
't/039_end_of_wal.pl',
't/040_standby_failover_slots_sync.pl',
+ 't/041_invalid_checkpoint_after_promote.pl',
],
},
}
diff --git a/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
new file mode 100644
index 0000000000..e4812d569d
--- /dev/null
+++ b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
@@ -0,0 +1,175 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Time::HiRes qw(usleep);
+use Test::More;
+
+##################################################
+# Test race condition when a restart point is running during a promotion,
+# checking that WAL segments are correctly removed in the restart point
+# while the promotion finishes.
+#
+# This test relies on an injection point that causes the checkpointer to
+# wait in the middle of a restart point on a standby. The checkpointer
+# is awaken to finish its restart point only once the promotion of the
+# standby is completed, and the node should be able to restart properly.
+##################################################
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('master');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', q[
+checkpoint_timeout = 30s
+log_checkpoints = on
+restart_after_crash = on
+]);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+# Setup a standby
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Dummy table for the upcoming tests.
+$node_primary->safe_psql('postgres', 'checkpoint');
+$node_primary->safe_psql('postgres', 'CREATE TABLE prim_tab (a int);');
+
+# Register a injection point on the standby so as the follow-up
+# restart point will wait on it.
+$node_primary->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# Wait until the extension has been created on the standby
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Note that from this point the checkpointer will wait in the middle of
+# a restart point on the standby.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_attach('CreateRestartPoint', 'wait');");
+
+# Execute a restart point on the standby, that we will now be waiting on.
+# This needs to be in the background.
+my $logstart = -s $node_standby->logfile;
+my $psql_session =
+ $node_standby->background_psql('postgres', on_error_stop => 0);
+$psql_session->query_until(
+ qr/starting_checkpoint/, q(
+ \echo starting_checkpoint
+ CHECKPOINT;
+));
+
+# Switch one WAL segment to make the previous restart point remove the
+# segment once the restart point completes.
+$node_primary->safe_psql('postgres', 'INSERT INTO prim_tab VALUES (1);');
+$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();');
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'injection_wait' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
+
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+my $log = slurp_file($node_standby->logfile, $logstart);
+my $checkpoint_start = 0;
+if ($log =~ m/restartpoint starting: immediate wait/)
+{
+ $checkpoint_start = 1;
+}
+is($checkpoint_start, 1, 'restartpoint has started');
+
+# Trigger promotion during the restart point.
+$node_primary->stop;
+$node_standby->promote;
+
+# Update the start position before waking up the checkpointer!
+$logstart = -s $node_standby->logfile;
+
+# Now wake up the checkpointer.
+$node_standby->safe_psql('postgres', "SELECT injection_points_wake();");
+
+# Wait until the previous restart point completes on the newly-promoted
+# standby, checking the logs for that.
+my $checkpoint_complete = 0;
+for (my $i = 0; $i < 3000; $i++)
+{
+ my $log = slurp_file($node_standby->logfile, $logstart);
+ if ($log =~ m/restartpoint complete/)
+ {
+ $checkpoint_complete = 1;
+ last;
+ }
+ usleep(100_000);
+}
+is($checkpoint_complete, 1, 'restart point has completed');
+
+# Kill with SIGKILL, forcing all the backends to restart.
+my $psql_timeout = IPC::Run::timer(3600);
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+ [
+ 'psql', '-XAtq', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+ $node_standby->connstr('postgres')
+ ],
+ '<',
+ \$killme_stdin,
+ '>',
+ \$killme_stdout,
+ '2>',
+ \$killme_stderr,
+ $psql_timeout);
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+$killme->pump until $killme_stdout =~ /[[:digit:]]+[\r\n]$/;
+my $pid = $killme_stdout;
+chomp($pid);
+$killme_stdout = '';
+$killme_stderr = '';
+
+my $ret = PostgreSQL::Test::Utils::system_log('pg_ctl', 'kill', 'KILL', $pid);
+is($ret, 0, 'killed process with KILL');
+
+# Wait until the server restarts, finish consuming output.
+$killme_stdin .= q[
+SELECT 1;
+];
+ok( pump_until(
+ $killme,
+ $psql_timeout,
+ \$killme_stderr,
+ qr/server closed the connection unexpectedly|connection to server was lost|could not send data to server/m
+ ),
+ "psql query died successfully after SIGKILL");
+$killme->finish;
+
+# Wait till server finishes restarting
+$node_standby->poll_query_until('postgres', undef, '');
+
+# After recovery, the server should be able to start.
+my $stdout;
+my $stderr;
+($ret, $stdout, $stderr) = $node_standby->psql('postgres', 'select 1');
+is($ret, 0, "psql connect success");
+is($stdout, 1, "psql select 1");
+
+done_testing();
--
2.43.0
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-19 14:28 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-19 14:28 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat@gmail.com>
Hi,
On Mon, Feb 19, 2024 at 04:51:45PM +0900, Michael Paquier wrote:
> On Mon, Feb 19, 2024 at 03:01:40PM +0900, Michael Paquier wrote:
> > 0002 is a polished version of the TAP test that makes use of this
> > facility, providing coverage for the bug fixed by 7863ee4def65
> > (reverting this commit causes the test to fail), where a restart point
> > runs across a promotion request. The trick is to stop the
> > checkpointer in the middle of a restart point and issue a promotion
> > in-between.
>
> The CF bot has been screaming at this one on Windows because the
> process started with IPC::Run::start was not properly finished, so
> attached is an updated version to bring that back to green.
Thanks for the patch, that's a very cool feature!
Random comments:
1 ===
+CREATE FUNCTION injection_points_wake()
what about injection_points_wakeup() instead?
2 ===
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* protects accesses to wait_counts */
+ slock_t lock;
+
+ /* Counter advancing when injection_points_wake() is called */
+ int wait_counts;
If slock_t protects "only" one counter, then what about using pg_atomic_uint64
or pg_atomic_uint32 instead? And btw do we need wait_counts at all? (see comment
number 4)
3 ===
+ * SQL function for waking a condition variable
waking up instead?
4 ===
+ for (;;)
+ {
+ int new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts;
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
I'm wondering if this loop and wait_counts are needed, why not doing something
like (and completely get rid of wait_counts)?
"
ConditionVariablePrepareToSleep(&inj_state->wait_point);
ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
ConditionVariableCancelSleep();
"
It's true that the comment above ConditionVariableSleep() mentions that:
"
* This should be called in a predicate loop that tests for a specific exit
* condition and otherwise sleeps
"
but is it needed in our particular case here?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-19 23:28 ` Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-19 23:28 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, Feb 19, 2024 at 02:28:04PM +0000, Bertrand Drouvot wrote:
> +CREATE FUNCTION injection_points_wake()
>
> what about injection_points_wakeup() instead?
Sure.
> +/* Shared state information for injection points. */
> +typedef struct InjectionPointSharedState
> +{
> + /* protects accesses to wait_counts */
> + slock_t lock;
> +
> + /* Counter advancing when injection_points_wake() is called */
> + int wait_counts;
>
> If slock_t protects "only" one counter, then what about using pg_atomic_uint64
> or pg_atomic_uint32 instead? And btw do we need wait_counts at all? (see comment
> number 4)
We could, indeed, even if we use more than one counter. Still, I
would be tempted to keep it in case more data is added to this
structure as that would make for less stuff to do while being normally
non-critical. This sentence may sound pedantic, though..
> + * SQL function for waking a condition variable
>
> waking up instead?
Okay.
> I'm wondering if this loop and wait_counts are needed, why not doing something
> like (and completely get rid of wait_counts)?
>
> "
> ConditionVariablePrepareToSleep(&inj_state->wait_point);
> ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
> ConditionVariableCancelSleep();
> "
>
> It's true that the comment above ConditionVariableSleep() mentions that:
Perhaps not, but it encourages good practices around the use of
condition variables, and we need to track all that in shared memory
anyway. Ashutosh has argued in favor of the approach taken by the
patch in the original thread when I've sent a version doing exactly
what you are saying now to not track a state in shmem.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-20 15:55 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-20 15:55 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Tue, Feb 20, 2024 at 08:28:28AM +0900, Michael Paquier wrote:
> On Mon, Feb 19, 2024 at 02:28:04PM +0000, Bertrand Drouvot wrote:
> > If slock_t protects "only" one counter, then what about using pg_atomic_uint64
> > or pg_atomic_uint32 instead? And btw do we need wait_counts at all? (see comment
> > number 4)
>
> We could, indeed, even if we use more than one counter. Still, I
> would be tempted to keep it in case more data is added to this
> structure as that would make for less stuff to do while being normally
> non-critical. This sentence may sound pedantic, though..
Okay, makes sense to keep this as it is as a "template" in case more stuff is
added.
+ /* Counter advancing when injection_points_wake() is called */
+ int wait_counts;
In that case what about using an unsigned instead? (Nit)
> > I'm wondering if this loop and wait_counts are needed, why not doing something
> > like (and completely get rid of wait_counts)?
> >
> > "
> > ConditionVariablePrepareToSleep(&inj_state->wait_point);
> > ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
> > ConditionVariableCancelSleep();
> > "
> >
> > It's true that the comment above ConditionVariableSleep() mentions that:
>
> Perhaps not, but it encourages good practices around the use of
> condition variables, and we need to track all that in shared memory
> anyway. Ashutosh has argued in favor of the approach taken by the
> patch in the original thread when I've sent a version doing exactly
> what you are saying now to not track a state in shmem.
Oh okay I missed this previous discussion, let's keep it as it is then.
New comments:
1 ===
+void
+injection_wait(const char *name)
Looks like name is not used in the function. I guess the reason it is a parameter
is because that's the way the callback function is being called in
InjectionPointRun()?
2 ===
+PG_FUNCTION_INFO_V1(injection_points_wake);
+Datum
+injection_points_wake(PG_FUNCTION_ARGS)
+{
I think that This function will wake up all the "wait" injection points.
Would that make sense to implement some filtering based on the name? That could
be useful for tests that would need multiple wait injection points and that want
to wake them up "sequentially".
We could think about it if there is such a need in the future though.
3 ===
+# Register a injection point on the standby so as the follow-up
typo: "an injection"?
4 ===
+for (my $i = 0; $i < 3000; $i++)
+{
is 3000 due to?:
+checkpoint_timeout = 30s
If so, would that make sense to reduce the value for both?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-20 22:08 ` Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 08:07 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 2 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-20 22:08 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Tue, Feb 20, 2024 at 03:55:08PM +0000, Bertrand Drouvot wrote:
> Okay, makes sense to keep this as it is as a "template" in case more stuff is
> added.
>
> + /* Counter advancing when injection_points_wake() is called */
> + int wait_counts;
>
> In that case what about using an unsigned instead? (Nit)
uint32. Sure.
> 1 ===
>
> +void
> +injection_wait(const char *name)
>
> Looks like name is not used in the function. I guess the reason it is a parameter
> is because that's the way the callback function is being called in
> InjectionPointRun()?
Right. The callback has to define this argument.
> 2 ===
>
> +PG_FUNCTION_INFO_V1(injection_points_wake);
> +Datum
> +injection_points_wake(PG_FUNCTION_ARGS)
> +{
>
> I think that This function will wake up all the "wait" injection points.
> Would that make sense to implement some filtering based on the name? That could
> be useful for tests that would need multiple wait injection points and that want
> to wake them up "sequentially".
>
> We could think about it if there is such a need in the future though.
Well, both you and Andrey are asking for it now, so let's do it. The
implementation is simple:
- Store in InjectionPointSharedState an array of wait_counts and an
array of names. There is only one condition variable.
- When a point wants to wait, it takes the spinlock and looks within
the array of names until it finds a free slot, adds its name into the
array to reserve a wait counter at the same position, releases the
spinlock. Then it loops on the condition variable for an update of
the counter it has reserved. It is possible to make something more
efficient, but at a small size it would not really matter.
- The wakeup takes a point name in argument, acquires the spinlock,
and checks if it can find the point into the array, pinpoints the
location of the counter to update and updates it. Then it broadcasts
the change.
- The wait loop checks its counter, leaves its loop, cancels the
sleep, takes the spinlock to unregister from the array, and leaves.
I would just hardcode the number of points that can wait, say 5 of
them tracked in shmem? Does that look like what you are looking at?
> +# Register a injection point on the standby so as the follow-up
>
> typo: "an injection"?
Oops. Fixed locally.
> +for (my $i = 0; $i < 3000; $i++)
> +{
>
> is 3000 due to?:
>
> +checkpoint_timeout = 30s
>
> If so, would that make sense to reduce the value for both?
That had better be based on PostgreSQL::Test::Utils::timeout_default,
actually, as in something like:
foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-21 07:46 ` Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
1 sibling, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-21 07:46 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> Well, both you and Andrey are asking for it now, so let's do it. The
> implementation is simple:
> - Store in InjectionPointSharedState an array of wait_counts and an
> array of names. There is only one condition variable.
> - When a point wants to wait, it takes the spinlock and looks within
> the array of names until it finds a free slot, adds its name into the
> array to reserve a wait counter at the same position, releases the
> spinlock. Then it loops on the condition variable for an update of
> the counter it has reserved. It is possible to make something more
> efficient, but at a small size it would not really matter.
> - The wakeup takes a point name in argument, acquires the spinlock,
> and checks if it can find the point into the array, pinpoints the
> location of the counter to update and updates it. Then it broadcasts
> the change.
> - The wait loop checks its counter, leaves its loop, cancels the
> sleep, takes the spinlock to unregister from the array, and leaves.
>
> I would just hardcode the number of points that can wait, say 5 of
> them tracked in shmem? Does that look like what you are looking at?
I was looking at that, and it proves to work OK, so you can do stuff
like waits and wakeups for multiple processes in a controlled manner.
The attached patch authorizes up to 32 waiters. I have switched
things so as the information reported in pg_stat_activity is the name
of the injection point itself.
Comments and ideas are welcome.
--
Michael
Attachments: download all (3 files, .tar.gz)
v2-0001-injection_points-Add-routines-to-wait-and-wake-pr.patch (text/x-diff, 7.1K) download
From 75302cba302b83ce2a6d6eaf30b163f473b87276 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 21 Feb 2024 16:36:25 +0900
Subject: [PATCH v2 1/2] injection_points: Add routines to wait and wake
processes
This commit is made of two parts:
- A new callback that can be attached to a process to make it wait on a
condition variable. The condition checked is registered in shared
memory by the module injection_points.
- A new SQL function to update the shared state and broadcast the update
using a condition variable.
The shared state used by the module is registered using the DSM
registry, and is optional.
---
.../injection_points--1.0.sql | 10 ++
.../injection_points/injection_points.c | 151 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 162 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..eed0310cf6 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -24,6 +24,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_wakeup()
+--
+-- Wakes a condition variable waited on in an injection point.
+--
+CREATE FUNCTION injection_points_wakeup(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_wakeup'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_detach()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..052b20f9c8 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,18 +18,72 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/condition_variable.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
+#include "storage/dsm_registry.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
#include "utils/wait_event.h"
PG_MODULE_MAGIC;
+/* Maximum number of wait usable in injection points at once */
+#define INJ_MAX_WAIT 32
+#define INJ_NAME_MAXLEN 64
+
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* protects accesses to wait_counts */
+ slock_t lock;
+
+ /* Counters advancing when injection_points_wakeup() is called */
+ int wait_counts[INJ_MAX_WAIT];
+
+ /* Names of injection points attached to wait counters */
+ char name[INJ_MAX_WAIT][INJ_NAME_MAXLEN];
+
+ /*
+ * Condition variable used for waits and wakeups, checking upon the set of
+ * wait_counts when waiting.
+ */
+ ConditionVariable wait_point;
+} InjectionPointSharedState;
+
+/* Pointer to shared-memory state. */
+static InjectionPointSharedState *inj_state = NULL;
+
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
+static void
+injection_point_init_state(void *ptr)
+{
+ InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
+
+ SpinLockInit(&state->lock);
+ memset(state->wait_counts, 0, sizeof(state->wait_counts));
+ memset(state->name, 0, sizeof(state->name));
+ ConditionVariableInit(&state->wait_point);
+}
+
+static void
+injection_init_shmem(void)
+{
+ bool found;
+
+ if (inj_state != NULL)
+ return;
+
+ inj_state = GetNamedDSMSegment("injection_points",
+ sizeof(InjectionPointSharedState),
+ injection_point_init_state,
+ &found);
+}
+
/* Set of callbacks available to be attached to an injection point. */
void
injection_error(const char *name)
@@ -43,6 +97,65 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+/* Wait on a condition variable, awaken by injection_points_wakeup() */
+void
+injection_wait(const char *name)
+{
+ int old_wait_counts = -1;
+ int index = -1;
+ uint32 injection_wait_event = 0;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /*
+ * This custom wait event name is not released, but we don't care much for
+ * testing as this will be short-lived.
+ */
+ injection_wait_event = WaitEventExtensionNew(name);
+
+ /*
+ * Find a free slot to wait for, and register this injection point's name.
+ */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (inj_state->name[i][0] == '\0')
+ {
+ index = i;
+ strlcpy(inj_state->name[i], name, INJ_NAME_MAXLEN);
+ old_wait_counts = inj_state->wait_counts[i];
+ break;
+ }
+ }
+ SpinLockRelease(&inj_state->lock);
+
+ if (index < 0)
+ elog(ERROR, "could not find free slot for wait of injection point %s ",
+ name);
+
+ /* And sleep.. */
+ ConditionVariablePrepareToSleep(&inj_state->wait_point);
+ for (;;)
+ {
+ int new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts[index];
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
+ ConditionVariableCancelSleep();
+
+ /* Remove us from the waiting list */
+ SpinLockAcquire(&inj_state->lock);
+ inj_state->name[index][0] = '\0';
+ SpinLockRelease(&inj_state->lock);
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +171,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
@@ -80,6 +195,42 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for waking a condition variable.
+ */
+PG_FUNCTION_INFO_V1(injection_points_wakeup);
+Datum
+injection_points_wakeup(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ int index = -1;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /* First bump the wait counter for the injection point to wake */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (strcmp(name, inj_state->name[i]) == 0)
+ {
+ index = i;
+ break;
+ }
+ }
+ if (index < 0)
+ {
+ SpinLockRelease(&inj_state->lock);
+ elog(ERROR, "could not find injection point %s to wake", name);
+ }
+ inj_state->wait_counts[index]++;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And broadcast the change for the waiters */
+ ConditionVariableBroadcast(&inj_state->wait_point);
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for dropping an injection point.
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d808aad8b0..d7eca00502 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1208,6 +1208,7 @@ InitializeDSMForeignScan_function
InitializeWorkerForeignScan_function
InjectionPointCacheEntry
InjectionPointEntry
+InjectionPointSharedState
InlineCodeBlock
InsertStmt
Instrumentation
--
2.43.0
v2-0002-Add-regression-test-for-restart-points-during-pro.patch (text/x-diff, 9.1K) download
From 79cc3e309c15bec19fec8c3cb2ac2e48bd42d7a2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 21 Feb 2024 16:37:17 +0900
Subject: [PATCH v2 2/2] Add regression test for restart points during
promotion
This test fails when 7863ee4def65 is reverted, checking that a node is
able to properly restart following a crash when a restart point was
finishing across a promotion.
This is an old bug that had no coverage, and injection points make that
cheap to achieve.
---
src/backend/access/transam/xlog.c | 7 +
src/test/recovery/Makefile | 7 +-
src/test/recovery/meson.build | 4 +
.../t/041_invalid_checkpoint_after_promote.pl | 176 ++++++++++++++++++
4 files changed, 193 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 50c347a679..50b045ff08 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@
#include "storage/sync.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/relmapper.h"
@@ -7536,6 +7537,12 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /*
+ * This location needs to be after CheckPointGuts() to ensure that some
+ * work has already happened during this checkpoint.
+ */
+ INJECTION_POINT("CreateRestartPoint");
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 17ee353735..f57baba5e8 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,12 +9,17 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm contrib/pg_stat_statements contrib/test_decoding
+EXTRA_INSTALL=contrib/pg_prewarm \
+ contrib/pg_stat_statements \
+ contrib/test_decoding \
+ src/test/modules/injection_points
subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export enable_injection_points enable_injection_points
+
# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index bf087ac2a9..e4e0e2b4cc 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -6,6 +6,9 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'test_kwargs': {'priority': 40}, # recovery tests are slow, start early
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_stream_rep.pl',
't/002_archiving.pl',
@@ -46,6 +49,7 @@ tests += {
't/038_save_logical_slots_shutdown.pl',
't/039_end_of_wal.pl',
't/040_standby_failover_slots_sync.pl',
+ 't/041_invalid_checkpoint_after_promote.pl',
],
},
}
diff --git a/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
new file mode 100644
index 0000000000..e91f360d12
--- /dev/null
+++ b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
@@ -0,0 +1,176 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Time::HiRes qw(usleep);
+use Test::More;
+
+##################################################
+# Test race condition when a restart point is running during a promotion,
+# checking that WAL segments are correctly removed in the restart point
+# while the promotion finishes.
+#
+# This test relies on an injection point that causes the checkpointer to
+# wait in the middle of a restart point on a standby. The checkpointer
+# is awaken to finish its restart point only once the promotion of the
+# standby is completed, and the node should be able to restart properly.
+##################################################
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('master');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', q[
+checkpoint_timeout = 30s
+log_checkpoints = on
+restart_after_crash = on
+]);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+# Setup a standby
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Dummy table for the upcoming tests.
+$node_primary->safe_psql('postgres', 'checkpoint');
+$node_primary->safe_psql('postgres', 'CREATE TABLE prim_tab (a int);');
+
+# Register an injection point on the standby so as the follow-up
+# restart point will wait on it.
+$node_primary->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# Wait until the extension has been created on the standby
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Note that from this point the checkpointer will wait in the middle of
+# a restart point on the standby.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_attach('CreateRestartPoint', 'wait');");
+
+# Execute a restart point on the standby, that we will now be waiting on.
+# This needs to be in the background.
+my $logstart = -s $node_standby->logfile;
+my $psql_session =
+ $node_standby->background_psql('postgres', on_error_stop => 0);
+$psql_session->query_until(
+ qr/starting_checkpoint/, q(
+ \echo starting_checkpoint
+ CHECKPOINT;
+));
+
+# Switch one WAL segment to make the previous restart point remove the
+# segment once the restart point completes.
+$node_primary->safe_psql('postgres', 'INSERT INTO prim_tab VALUES (1);');
+$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();');
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
+
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+my $log = slurp_file($node_standby->logfile, $logstart);
+my $checkpoint_start = 0;
+if ($log =~ m/restartpoint starting: immediate wait/)
+{
+ $checkpoint_start = 1;
+}
+is($checkpoint_start, 1, 'restartpoint has started');
+
+# Trigger promotion during the restart point.
+$node_primary->stop;
+$node_standby->promote;
+
+# Update the start position before waking up the checkpointer!
+$logstart = -s $node_standby->logfile;
+
+# Now wake up the checkpointer.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_wakeup('CreateRestartPoint');");
+
+# Wait until the previous restart point completes on the newly-promoted
+# standby, checking the logs for that.
+my $checkpoint_complete = 0;
+foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
+{
+ my $log = slurp_file($node_standby->logfile, $logstart);
+ if ($log =~ m/restartpoint complete/)
+ {
+ $checkpoint_complete = 1;
+ last;
+ }
+ usleep(100_000);
+}
+is($checkpoint_complete, 1, 'restart point has completed');
+
+# Kill with SIGKILL, forcing all the backends to restart.
+my $psql_timeout = IPC::Run::timer(3600);
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+ [
+ 'psql', '-XAtq', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+ $node_standby->connstr('postgres')
+ ],
+ '<',
+ \$killme_stdin,
+ '>',
+ \$killme_stdout,
+ '2>',
+ \$killme_stderr,
+ $psql_timeout);
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+$killme->pump until $killme_stdout =~ /[[:digit:]]+[\r\n]$/;
+my $pid = $killme_stdout;
+chomp($pid);
+$killme_stdout = '';
+$killme_stderr = '';
+
+my $ret = PostgreSQL::Test::Utils::system_log('pg_ctl', 'kill', 'KILL', $pid);
+is($ret, 0, 'killed process with KILL');
+
+# Wait until the server restarts, finish consuming output.
+$killme_stdin .= q[
+SELECT 1;
+];
+ok( pump_until(
+ $killme,
+ $psql_timeout,
+ \$killme_stderr,
+ qr/server closed the connection unexpectedly|connection to server was lost|could not send data to server/m
+ ),
+ "psql query died successfully after SIGKILL");
+$killme->finish;
+
+# Wait till server finishes restarting
+$node_standby->poll_query_until('postgres', undef, '');
+
+# After recovery, the server should be able to start.
+my $stdout;
+my $stderr;
+($ret, $stdout, $stderr) = $node_standby->psql('postgres', 'select 1');
+is($ret, 0, "psql connect success");
+is($stdout, 1, "psql select 1");
+
+done_testing();
--
2.43.0
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-21 11:50 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-21 11:50 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Wed, Feb 21, 2024 at 04:46:00PM +0900, Michael Paquier wrote:
> On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> > Well, both you and Andrey are asking for it now, so let's do it. The
> > implementation is simple:
> > - Store in InjectionPointSharedState an array of wait_counts and an
> > array of names. There is only one condition variable.
> > - When a point wants to wait, it takes the spinlock and looks within
> > the array of names until it finds a free slot, adds its name into the
> > array to reserve a wait counter at the same position, releases the
> > spinlock. Then it loops on the condition variable for an update of
> > the counter it has reserved. It is possible to make something more
> > efficient, but at a small size it would not really matter.
> > - The wakeup takes a point name in argument, acquires the spinlock,
> > and checks if it can find the point into the array, pinpoints the
> > location of the counter to update and updates it. Then it broadcasts
> > the change.
> > - The wait loop checks its counter, leaves its loop, cancels the
> > sleep, takes the spinlock to unregister from the array, and leaves.
> >
> > I would just hardcode the number of points that can wait, say 5 of
> > them tracked in shmem? Does that look like what you are looking at?
>
> I was looking at that, and it proves to work OK, so you can do stuff
> like waits and wakeups for multiple processes in a controlled manner.
> The attached patch authorizes up to 32 waiters. I have switched
> things so as the information reported in pg_stat_activity is the name
> of the injection point itself.
Thanks!
I think the approach is fine and the hardcoded value is "large" enough (it would
be surprising, at least to me, to write a test that would need more than 32
waiters).
A few comments:
1 ===
+-- Wakes a condition variable
I think "up" is missing at several places in the patch where "wake" is used.
I could be wrong as non native english speaker though.
2 ===
+ /* Counters advancing when injection_points_wakeup() is called */
+ int wait_counts[INJ_MAX_WAIT];
uint32? (here and other places where counter is manipulated).
3 ===
+ /* Remove us from the waiting list */
"Remove this injection wait name from the waiting list" instead?
4 ===
+ * SQL function for waking a condition variable.
s/a condition variable/an injection wait point/ ?
5 ===
+PG_FUNCTION_INFO_V1(injection_points_wakeup);
+Datum
+injection_points_wakeup(PG_FUNCTION_ARGS)
Empty line missing before "Datum"?
6 ===
Also maybe some comments are missing above injection_point_init_state(),
injection_init_shmem() but it's more a Nit.
7 ===
While at it, should we add a second injection wait point in
t/041_invalid_checkpoint_after_promote.pl to check that they are wake up
individually?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-22 03:02 ` Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-22 03:02 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, Feb 21, 2024 at 11:50:21AM +0000, Bertrand Drouvot wrote:
> I think the approach is fine and the hardcoded value is "large" enough (it would
> be surprising, at least to me, to write a test that would need more than 32
> waiters).
This could use less. I've never used more than 3 of them in a single
test, and that was already very complex to follow.
> A few comments:
>
> 1 ===
> I think "up" is missing at several places in the patch where "wake" is used.
> I could be wrong as non native english speaker though.
Patched up three places to be more consisten.
> 2 ===
>
> + /* Counters advancing when injection_points_wakeup() is called */
> + int wait_counts[INJ_MAX_WAIT];
>
> uint32? (here and other places where counter is manipulated).
Okay, why not.
> "Remove this injection wait name from the waiting list" instead?
> s/a condition variable/an injection wait point/ ?
Okay.
> 5 ===
>
> +PG_FUNCTION_INFO_V1(injection_points_wakeup);
> +Datum
> +injection_points_wakeup(PG_FUNCTION_ARGS)
>
> Empty line missing before "Datum"?
I've used the same style as anywhere else.
> Also maybe some comments are missing above injection_point_init_state(),
> injection_init_shmem() but it's more a Nit.
Sure.
> While at it, should we add a second injection wait point in
> t/041_invalid_checkpoint_after_promote.pl to check that they are wake up
> individually?
I'm not sure that's a good idea for the sake of this test, TBH, as
that would provide coverage outside the original scope of the
restartpoint/promote check.
I have also looked at if it would be possible to get an isolation test
out of that, but the arbitrary wait does not make that possible with
the existing isolation APIs, see GetSafeSnapshotBlockingPids() used in
pg_isolation_test_session_is_blocked(). isolation/README seems to be
a bit off here, actually, mentioning pg_locks.. We could use some
tricks with transactions or locks internally, but I'm not really
tempted to make the wait callback more complicated for the sake of
more coverage as I'd rather keep it generic for anybody who wants to
control the order of events across processes.
Attaching a v3 for reference with everything that has been mentioned
up to now.
--
Michael
Attachments: download all (3 files, .tar.gz)
v3-0001-injection_points-Add-routines-to-wait-and-wake-pr.patch (text/x-diff, 7.2K) download
From 0ccbacd01b326078aa750f1a72f38823f8f9c8b3 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 22 Feb 2024 11:53:29 +0900
Subject: [PATCH v3 1/2] injection_points: Add routines to wait and wake
processes
This commit is made of two parts:
- A new callback that can be attached to a process to make it wait on a
condition variable. The condition checked is registered in shared
memory by the module injection_points.
- A new SQL function to update the shared state and broadcast the update
using a condition variable.
The shared state used by the module is registered using the DSM
registry, and is optional.
---
.../injection_points--1.0.sql | 10 ++
.../injection_points/injection_points.c | 155 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 166 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..0a2e59aba7 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -24,6 +24,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_wakeup()
+--
+-- Wakes up a waiting injection point.
+--
+CREATE FUNCTION injection_points_wakeup(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_wakeup'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_detach()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..142211d0d7 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,18 +18,76 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/condition_variable.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
+#include "storage/dsm_registry.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
#include "utils/wait_event.h"
PG_MODULE_MAGIC;
+/* Maximum number of wait usable in injection points at once */
+#define INJ_MAX_WAIT 32
+#define INJ_NAME_MAXLEN 64
+
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* protects accesses to wait_counts */
+ slock_t lock;
+
+ /* Counters advancing when injection_points_wakeup() is called */
+ uint32 wait_counts[INJ_MAX_WAIT];
+
+ /* Names of injection points attached to wait counters */
+ char name[INJ_MAX_WAIT][INJ_NAME_MAXLEN];
+
+ /*
+ * Condition variable used for waits and wakeups, checking upon the set of
+ * wait_counts when waiting.
+ */
+ ConditionVariable wait_point;
+} InjectionPointSharedState;
+
+/* Pointer to shared-memory state. */
+static InjectionPointSharedState *inj_state = NULL;
+
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
+/* Callback for shmem area initialization */
+static void
+injection_point_init_state(void *ptr)
+{
+ InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
+
+ SpinLockInit(&state->lock);
+ memset(state->wait_counts, 0, sizeof(state->wait_counts));
+ memset(state->name, 0, sizeof(state->name));
+ ConditionVariableInit(&state->wait_point);
+}
+
+/*
+ * Initialize shared memory area for this module.
+ */
+static void
+injection_init_shmem(void)
+{
+ bool found;
+
+ if (inj_state != NULL)
+ return;
+
+ inj_state = GetNamedDSMSegment("injection_points",
+ sizeof(InjectionPointSharedState),
+ injection_point_init_state,
+ &found);
+}
+
/* Set of callbacks available to be attached to an injection point. */
void
injection_error(const char *name)
@@ -43,6 +101,65 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+/* Wait on a condition variable, awaken by injection_points_wakeup() */
+void
+injection_wait(const char *name)
+{
+ uint32 old_wait_counts = 0;
+ int index = -1;
+ uint32 injection_wait_event = 0;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /*
+ * This custom wait event name is not released, but we don't care much for
+ * testing as this will be short-lived.
+ */
+ injection_wait_event = WaitEventExtensionNew(name);
+
+ /*
+ * Find a free slot to wait for, and register this injection point's name.
+ */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (inj_state->name[i][0] == '\0')
+ {
+ index = i;
+ strlcpy(inj_state->name[i], name, INJ_NAME_MAXLEN);
+ old_wait_counts = inj_state->wait_counts[i];
+ break;
+ }
+ }
+ SpinLockRelease(&inj_state->lock);
+
+ if (index < 0)
+ elog(ERROR, "could not find free slot for wait of injection point %s ",
+ name);
+
+ /* And sleep.. */
+ ConditionVariablePrepareToSleep(&inj_state->wait_point);
+ for (;;)
+ {
+ uint32 new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts[index];
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
+ ConditionVariableCancelSleep();
+
+ /* Remove this injection point from the waiters. */
+ SpinLockAcquire(&inj_state->lock);
+ inj_state->name[index][0] = '\0';
+ SpinLockRelease(&inj_state->lock);
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +175,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
@@ -80,6 +199,42 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for waking up an injection point waiting in injection_wait().
+ */
+PG_FUNCTION_INFO_V1(injection_points_wakeup);
+Datum
+injection_points_wakeup(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ int index = -1;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /* First bump the wait counter for the injection point to wake up */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (strcmp(name, inj_state->name[i]) == 0)
+ {
+ index = i;
+ break;
+ }
+ }
+ if (index < 0)
+ {
+ SpinLockRelease(&inj_state->lock);
+ elog(ERROR, "could not find injection point %s to wake up", name);
+ }
+ inj_state->wait_counts[index]++;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And broadcast the change for the waiters */
+ ConditionVariableBroadcast(&inj_state->wait_point);
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for dropping an injection point.
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index d808aad8b0..d7eca00502 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1208,6 +1208,7 @@ InitializeDSMForeignScan_function
InitializeWorkerForeignScan_function
InjectionPointCacheEntry
InjectionPointEntry
+InjectionPointSharedState
InlineCodeBlock
InsertStmt
Instrumentation
--
2.43.0
v3-0002-Add-regression-test-for-restart-points-during-pro.patch (text/x-diff, 9.1K) download
From 4369505ad2719603a8e9b3ed0764e81c09891992 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 21 Feb 2024 16:37:17 +0900
Subject: [PATCH v3 2/2] Add regression test for restart points during
promotion
This test fails when 7863ee4def65 is reverted, checking that a node is
able to properly restart following a crash when a restart point was
finishing across a promotion.
This is an old bug that had no coverage, and injection points make that
cheap to achieve.
---
src/backend/access/transam/xlog.c | 7 +
src/test/recovery/Makefile | 7 +-
src/test/recovery/meson.build | 4 +
.../t/041_invalid_checkpoint_after_promote.pl | 176 ++++++++++++++++++
4 files changed, 193 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c1162d55bf..dbf820f988 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@
#include "storage/sync.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/relmapper.h"
@@ -7536,6 +7537,12 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /*
+ * This location needs to be after CheckPointGuts() to ensure that some
+ * work has already happened during this checkpoint.
+ */
+ INJECTION_POINT("CreateRestartPoint");
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 17ee353735..f57baba5e8 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,12 +9,17 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm contrib/pg_stat_statements contrib/test_decoding
+EXTRA_INSTALL=contrib/pg_prewarm \
+ contrib/pg_stat_statements \
+ contrib/test_decoding \
+ src/test/modules/injection_points
subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export enable_injection_points enable_injection_points
+
# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index bf087ac2a9..e4e0e2b4cc 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -6,6 +6,9 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'test_kwargs': {'priority': 40}, # recovery tests are slow, start early
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_stream_rep.pl',
't/002_archiving.pl',
@@ -46,6 +49,7 @@ tests += {
't/038_save_logical_slots_shutdown.pl',
't/039_end_of_wal.pl',
't/040_standby_failover_slots_sync.pl',
+ 't/041_invalid_checkpoint_after_promote.pl',
],
},
}
diff --git a/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
new file mode 100644
index 0000000000..e91f360d12
--- /dev/null
+++ b/src/test/recovery/t/041_invalid_checkpoint_after_promote.pl
@@ -0,0 +1,176 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Time::HiRes qw(usleep);
+use Test::More;
+
+##################################################
+# Test race condition when a restart point is running during a promotion,
+# checking that WAL segments are correctly removed in the restart point
+# while the promotion finishes.
+#
+# This test relies on an injection point that causes the checkpointer to
+# wait in the middle of a restart point on a standby. The checkpointer
+# is awaken to finish its restart point only once the promotion of the
+# standby is completed, and the node should be able to restart properly.
+##################################################
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('master');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', q[
+checkpoint_timeout = 30s
+log_checkpoints = on
+restart_after_crash = on
+]);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+# Setup a standby
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Dummy table for the upcoming tests.
+$node_primary->safe_psql('postgres', 'checkpoint');
+$node_primary->safe_psql('postgres', 'CREATE TABLE prim_tab (a int);');
+
+# Register an injection point on the standby so as the follow-up
+# restart point will wait on it.
+$node_primary->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# Wait until the extension has been created on the standby
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Note that from this point the checkpointer will wait in the middle of
+# a restart point on the standby.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_attach('CreateRestartPoint', 'wait');");
+
+# Execute a restart point on the standby, that we will now be waiting on.
+# This needs to be in the background.
+my $logstart = -s $node_standby->logfile;
+my $psql_session =
+ $node_standby->background_psql('postgres', on_error_stop => 0);
+$psql_session->query_until(
+ qr/starting_checkpoint/, q(
+ \echo starting_checkpoint
+ CHECKPOINT;
+));
+
+# Switch one WAL segment to make the previous restart point remove the
+# segment once the restart point completes.
+$node_primary->safe_psql('postgres', 'INSERT INTO prim_tab VALUES (1);');
+$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();');
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
+
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+my $log = slurp_file($node_standby->logfile, $logstart);
+my $checkpoint_start = 0;
+if ($log =~ m/restartpoint starting: immediate wait/)
+{
+ $checkpoint_start = 1;
+}
+is($checkpoint_start, 1, 'restartpoint has started');
+
+# Trigger promotion during the restart point.
+$node_primary->stop;
+$node_standby->promote;
+
+# Update the start position before waking up the checkpointer!
+$logstart = -s $node_standby->logfile;
+
+# Now wake up the checkpointer.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_wakeup('CreateRestartPoint');");
+
+# Wait until the previous restart point completes on the newly-promoted
+# standby, checking the logs for that.
+my $checkpoint_complete = 0;
+foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
+{
+ my $log = slurp_file($node_standby->logfile, $logstart);
+ if ($log =~ m/restartpoint complete/)
+ {
+ $checkpoint_complete = 1;
+ last;
+ }
+ usleep(100_000);
+}
+is($checkpoint_complete, 1, 'restart point has completed');
+
+# Kill with SIGKILL, forcing all the backends to restart.
+my $psql_timeout = IPC::Run::timer(3600);
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+ [
+ 'psql', '-XAtq', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+ $node_standby->connstr('postgres')
+ ],
+ '<',
+ \$killme_stdin,
+ '>',
+ \$killme_stdout,
+ '2>',
+ \$killme_stderr,
+ $psql_timeout);
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+$killme->pump until $killme_stdout =~ /[[:digit:]]+[\r\n]$/;
+my $pid = $killme_stdout;
+chomp($pid);
+$killme_stdout = '';
+$killme_stderr = '';
+
+my $ret = PostgreSQL::Test::Utils::system_log('pg_ctl', 'kill', 'KILL', $pid);
+is($ret, 0, 'killed process with KILL');
+
+# Wait until the server restarts, finish consuming output.
+$killme_stdin .= q[
+SELECT 1;
+];
+ok( pump_until(
+ $killme,
+ $psql_timeout,
+ \$killme_stderr,
+ qr/server closed the connection unexpectedly|connection to server was lost|could not send data to server/m
+ ),
+ "psql query died successfully after SIGKILL");
+$killme->finish;
+
+# Wait till server finishes restarting
+$node_standby->poll_query_until('postgres', undef, '');
+
+# After recovery, the server should be able to start.
+my $stdout;
+my $stderr;
+($ret, $stdout, $stderr) = $node_standby->psql('postgres', 'select 1');
+is($ret, 0, "psql connect success");
+is($stdout, 1, "psql select 1");
+
+done_testing();
--
2.43.0
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-22 08:00 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-22 08:00 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Thu, Feb 22, 2024 at 12:02:01PM +0900, Michael Paquier wrote:
> On Wed, Feb 21, 2024 at 11:50:21AM +0000, Bertrand Drouvot wrote:
> > A few comments:
> >
> > 1 ===
> > I think "up" is missing at several places in the patch where "wake" is used.
> > I could be wrong as non native english speaker though.
>
> Patched up three places to be more consisten.
Thanks!
> > 5 ===
> >
> > +PG_FUNCTION_INFO_V1(injection_points_wakeup);
> > +Datum
> > +injection_points_wakeup(PG_FUNCTION_ARGS)
> >
> > Empty line missing before "Datum"?
>
> I've used the same style as anywhere else.
humm, looking at src/test/regress/regress.c for example, I can see an empty
line between PG_FUNCTION_INFO_V1 and Datum for all the "PG_FUNCTION_INFO_V1"
ones.
> > While at it, should we add a second injection wait point in
> > t/041_invalid_checkpoint_after_promote.pl to check that they are wake up
> > individually?
>
> I'm not sure that's a good idea for the sake of this test, TBH, as
> that would provide coverage outside the original scope of the
> restartpoint/promote check.
Yeah, that was my doubt too.
> I have also looked at if it would be possible to get an isolation test
> out of that, but the arbitrary wait does not make that possible with
> the existing isolation APIs, see GetSafeSnapshotBlockingPids() used in
> pg_isolation_test_session_is_blocked(). isolation/README seems to be
> a bit off here, actually, mentioning pg_locks.. We could use some
> tricks with transactions or locks internally, but I'm not really
> tempted to make the wait callback more complicated for the sake of
> more coverage as I'd rather keep it generic for anybody who wants to
> control the order of events across processes.
Makes sense to me, let's keep it as it is.
>
> Attaching a v3 for reference with everything that has been mentioned
> up to now.
Thanks!
Sorry, I missed those ones previously:
1 ===
+/* Maximum number of wait usable in injection points at once */
s/Maximum number of wait/Maximum number of waits/ ?
2 ===
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+my $log = slurp_file($node_standby->logfile, $logstart);
+my $checkpoint_start = 0;
+if ($log =~ m/restartpoint starting: immediate wait/)
+{
+ $checkpoint_start = 1;
+}
+is($checkpoint_start, 1, 'restartpoint has started');
what about?
ok( $node_standby->log_contains( "restartpoint starting: immediate wait", $logstart),
"restartpoint has started");
Except for the above, v3 looks good to me.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-26 03:57 ` Michael Paquier <michael@paquier.xyz>
2024-02-26 08:24 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 2 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-26 03:57 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Thu, Feb 22, 2024 at 08:00:24AM +0000, Bertrand Drouvot wrote:
> +/* Maximum number of wait usable in injection points at once */
>
> s/Maximum number of wait/Maximum number of waits/ ?
Thanks. I've edited a few more places while scanning the whole.
>
> 2 ===
>
> +# Check the logs that the restart point has started on standby. This is
> +# optional, but let's be sure.
> +my $log = slurp_file($node_standby->logfile, $logstart);
> +my $checkpoint_start = 0;
> +if ($log =~ m/restartpoint starting: immediate wait/)
> +{
> + $checkpoint_start = 1;
> +}
> +is($checkpoint_start, 1, 'restartpoint has started');
>
> what about?
>
> ok( $node_standby->log_contains( "restartpoint starting: immediate wait", $logstart),
> "restartpoint has started");
And I'm behind the commit that introduced it (392ea0c78fdb). It is
possible to remove the dependency to slurp_file() entirely by
switching the second location checking the logs for the checkpoint
completion.
> Except for the above, v3 looks good to me.
Thanks. I'm looking at applying that at the beginning of next week if
there are no more comments, to get something by the feature freeze.
We could be more flexible for all that as that's related to testing,
but let's be in the clear.
I've also renamed the test to 041_checkpoint_at_promote.pl, as now
that the original is fixed, the checkpoint is not invalid. That's
cleaner this way.
--
Michael
Attachments: download all (3 files, .tar.gz)
v4-0001-injection_points-Add-routines-to-wait-and-wake-pr.patch (text/x-diff, 7.3K) download
From 1da8e77398bdb2e3a2c8a1d14a88c991fcae7919 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 26 Feb 2024 12:44:36 +0900
Subject: [PATCH v4 1/2] injection_points: Add routines to wait and wake
processes
This commit is made of two parts:
- A new callback that can be attached to a process to make it wait on a
condition variable. The condition checked is registered in shared
memory by the module injection_points.
- A new SQL function to update the shared state and broadcast the update
using a condition variable.
The shared state used by the module is registered using the DSM
registry, and is optional.
---
.../injection_points--1.0.sql | 10 ++
.../injection_points/injection_points.c | 155 ++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
3 files changed, 166 insertions(+)
diff --git a/src/test/modules/injection_points/injection_points--1.0.sql b/src/test/modules/injection_points/injection_points--1.0.sql
index 5944c41716..0a2e59aba7 100644
--- a/src/test/modules/injection_points/injection_points--1.0.sql
+++ b/src/test/modules/injection_points/injection_points--1.0.sql
@@ -24,6 +24,16 @@ RETURNS void
AS 'MODULE_PATHNAME', 'injection_points_run'
LANGUAGE C STRICT PARALLEL UNSAFE;
+--
+-- injection_points_wakeup()
+--
+-- Wakes up a waiting injection point.
+--
+CREATE FUNCTION injection_points_wakeup(IN point_name TEXT)
+RETURNS void
+AS 'MODULE_PATHNAME', 'injection_points_wakeup'
+LANGUAGE C STRICT PARALLEL UNSAFE;
+
--
-- injection_points_detach()
--
diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c
index e843e6594f..255941d105 100644
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -18,18 +18,75 @@
#include "postgres.h"
#include "fmgr.h"
+#include "storage/condition_variable.h"
#include "storage/lwlock.h"
#include "storage/shmem.h"
+#include "storage/dsm_registry.h"
#include "utils/builtins.h"
#include "utils/injection_point.h"
#include "utils/wait_event.h"
PG_MODULE_MAGIC;
+/* Maximum number of waits usable in injection points at once */
+#define INJ_MAX_WAIT 32
+#define INJ_NAME_MAXLEN 64
+
+/* Shared state information for injection points. */
+typedef struct InjectionPointSharedState
+{
+ /* Protects access to other fields */
+ slock_t lock;
+
+ /* Counters advancing when injection_points_wakeup() is called */
+ uint32 wait_counts[INJ_MAX_WAIT];
+
+ /* Names of injection points attached to wait counters */
+ char name[INJ_MAX_WAIT][INJ_NAME_MAXLEN];
+
+ /* Condition variable used for waits and wakeups */
+ ConditionVariable wait_point;
+} InjectionPointSharedState;
+
+/* Pointer to shared-memory state. */
+static InjectionPointSharedState *inj_state = NULL;
+
extern PGDLLEXPORT void injection_error(const char *name);
extern PGDLLEXPORT void injection_notice(const char *name);
+extern PGDLLEXPORT void injection_wait(const char *name);
+/*
+ * Callback for shared memory area initialization.
+ */
+static void
+injection_point_init_state(void *ptr)
+{
+ InjectionPointSharedState *state = (InjectionPointSharedState *) ptr;
+
+ SpinLockInit(&state->lock);
+ memset(state->wait_counts, 0, sizeof(state->wait_counts));
+ memset(state->name, 0, sizeof(state->name));
+ ConditionVariableInit(&state->wait_point);
+}
+
+/*
+ * Initialize shared memory area for this module.
+ */
+static void
+injection_init_shmem(void)
+{
+ bool found;
+
+ if (inj_state != NULL)
+ return;
+
+ inj_state = GetNamedDSMSegment("injection_points",
+ sizeof(InjectionPointSharedState),
+ injection_point_init_state,
+ &found);
+}
+
/* Set of callbacks available to be attached to an injection point. */
void
injection_error(const char *name)
@@ -43,6 +100,66 @@ injection_notice(const char *name)
elog(NOTICE, "notice triggered for injection point %s", name);
}
+/* Wait on a condition variable, awaken by injection_points_wakeup() */
+void
+injection_wait(const char *name)
+{
+ uint32 old_wait_counts = 0;
+ int index = -1;
+ uint32 injection_wait_event = 0;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /*
+ * Use the injection point name for this custom wait event. Note that
+ * this custom wait event name is not released, but we don't care much for
+ * testing as this should be short-lived.
+ */
+ injection_wait_event = WaitEventExtensionNew(name);
+
+ /*
+ * Find a free slot to wait for, and register this injection point's name.
+ */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (inj_state->name[i][0] == '\0')
+ {
+ index = i;
+ strlcpy(inj_state->name[i], name, INJ_NAME_MAXLEN);
+ old_wait_counts = inj_state->wait_counts[i];
+ break;
+ }
+ }
+ SpinLockRelease(&inj_state->lock);
+
+ if (index < 0)
+ elog(ERROR, "could not find free slot for wait of injection point %s ",
+ name);
+
+ /* And sleep.. */
+ ConditionVariablePrepareToSleep(&inj_state->wait_point);
+ for (;;)
+ {
+ uint32 new_wait_counts;
+
+ SpinLockAcquire(&inj_state->lock);
+ new_wait_counts = inj_state->wait_counts[index];
+ SpinLockRelease(&inj_state->lock);
+
+ if (old_wait_counts != new_wait_counts)
+ break;
+ ConditionVariableSleep(&inj_state->wait_point, injection_wait_event);
+ }
+ ConditionVariableCancelSleep();
+
+ /* Remove this injection point from the waiters. */
+ SpinLockAcquire(&inj_state->lock);
+ inj_state->name[index][0] = '\0';
+ SpinLockRelease(&inj_state->lock);
+}
+
/*
* SQL function for creating an injection point.
*/
@@ -58,6 +175,8 @@ injection_points_attach(PG_FUNCTION_ARGS)
function = "injection_error";
else if (strcmp(action, "notice") == 0)
function = "injection_notice";
+ else if (strcmp(action, "wait") == 0)
+ function = "injection_wait";
else
elog(ERROR, "incorrect action \"%s\" for injection point creation", action);
@@ -80,6 +199,42 @@ injection_points_run(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+/*
+ * SQL function for waking up an injection point waiting in injection_wait().
+ */
+PG_FUNCTION_INFO_V1(injection_points_wakeup);
+Datum
+injection_points_wakeup(PG_FUNCTION_ARGS)
+{
+ char *name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ int index = -1;
+
+ if (inj_state == NULL)
+ injection_init_shmem();
+
+ /* First bump the wait counter for the injection point to wake up */
+ SpinLockAcquire(&inj_state->lock);
+ for (int i = 0; i < INJ_MAX_WAIT; i++)
+ {
+ if (strcmp(name, inj_state->name[i]) == 0)
+ {
+ index = i;
+ break;
+ }
+ }
+ if (index < 0)
+ {
+ SpinLockRelease(&inj_state->lock);
+ elog(ERROR, "could not find injection point %s to wake up", name);
+ }
+ inj_state->wait_counts[index]++;
+ SpinLockRelease(&inj_state->lock);
+
+ /* And broadcast the change to the waiters */
+ ConditionVariableBroadcast(&inj_state->wait_point);
+ PG_RETURN_VOID();
+}
+
/*
* SQL function for dropping an injection point.
*/
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index fc8b15d0cf..4742784ea3 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1210,6 +1210,7 @@ InitializeDSMForeignScan_function
InitializeWorkerForeignScan_function
InjectionPointCacheEntry
InjectionPointEntry
+InjectionPointSharedState
InlineCodeBlock
InsertStmt
Instrumentation
--
2.43.0
v4-0002-Add-regression-test-for-restart-points-during-pro.patch (text/x-diff, 9.0K) download
From a86fd5268953372ada45b393cffc426ff3d6f12e Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 26 Feb 2024 12:55:36 +0900
Subject: [PATCH v4 2/2] Add regression test for restart points during
promotion
This test fails when 7863ee4def65 is reverted, checking that a node is
able to properly restart following a crash when a restart point was
finishing across a promotion.
This is an old bug that had no coverage, and injection points make that
cheap to achieve.
---
src/backend/access/transam/xlog.c | 7 +
src/test/recovery/Makefile | 7 +-
src/test/recovery/meson.build | 4 +
.../recovery/t/041_checkpoint_at_promote.pl | 171 ++++++++++++++++++
4 files changed, 188 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/041_checkpoint_at_promote.pl
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c1162d55bf..dbf820f988 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -100,6 +100,7 @@
#include "storage/sync.h"
#include "utils/guc_hooks.h"
#include "utils/guc_tables.h"
+#include "utils/injection_point.h"
#include "utils/memutils.h"
#include "utils/ps_status.h"
#include "utils/relmapper.h"
@@ -7536,6 +7537,12 @@ CreateRestartPoint(int flags)
CheckPointGuts(lastCheckPoint.redo, flags);
+ /*
+ * This location needs to be after CheckPointGuts() to ensure that some
+ * work has already happened during this checkpoint.
+ */
+ INJECTION_POINT("CreateRestartPoint");
+
/*
* Remember the prior checkpoint's redo ptr for
* UpdateCheckPointDistanceEstimate()
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 17ee353735..f57baba5e8 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -9,12 +9,17 @@
#
#-------------------------------------------------------------------------
-EXTRA_INSTALL=contrib/pg_prewarm contrib/pg_stat_statements contrib/test_decoding
+EXTRA_INSTALL=contrib/pg_prewarm \
+ contrib/pg_stat_statements \
+ contrib/test_decoding \
+ src/test/modules/injection_points
subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
+export enable_injection_points enable_injection_points
+
# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build
index bf087ac2a9..c67249500e 100644
--- a/src/test/recovery/meson.build
+++ b/src/test/recovery/meson.build
@@ -6,6 +6,9 @@ tests += {
'bd': meson.current_build_dir(),
'tap': {
'test_kwargs': {'priority': 40}, # recovery tests are slow, start early
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_stream_rep.pl',
't/002_archiving.pl',
@@ -46,6 +49,7 @@ tests += {
't/038_save_logical_slots_shutdown.pl',
't/039_end_of_wal.pl',
't/040_standby_failover_slots_sync.pl',
+ 't/041_checkpoint_at_promote.pl',
],
},
}
diff --git a/src/test/recovery/t/041_checkpoint_at_promote.pl b/src/test/recovery/t/041_checkpoint_at_promote.pl
new file mode 100644
index 0000000000..47381a2c82
--- /dev/null
+++ b/src/test/recovery/t/041_checkpoint_at_promote.pl
@@ -0,0 +1,171 @@
+
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Time::HiRes qw(usleep);
+use Test::More;
+
+##################################################
+# Test race condition when a restart point is running during a promotion,
+# checking that WAL segments are correctly removed in the restart point
+# while the promotion finishes.
+#
+# This test relies on an injection point that causes the checkpointer to
+# wait in the middle of a restart point on a standby. The checkpointer
+# is awaken to finish its restart point only once the promotion of the
+# standby is completed, and the node should be able to restart properly.
+##################################################
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('master');
+$node_primary->init(allows_streaming => 1);
+$node_primary->append_conf(
+ 'postgresql.conf', q[
+checkpoint_timeout = 30s
+log_checkpoints = on
+restart_after_crash = on
+]);
+$node_primary->start;
+
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+# Setup a standby
+my $node_standby = PostgreSQL::Test::Cluster->new('standby1');
+$node_standby->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby->start;
+
+# Dummy table for the upcoming tests.
+$node_primary->safe_psql('postgres', 'checkpoint');
+$node_primary->safe_psql('postgres', 'CREATE TABLE prim_tab (a int);');
+
+# Register an injection point on the standby so as the follow-up
+# restart point will wait on it.
+$node_primary->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# Wait until the extension has been created on the standby
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Note that from this point the checkpointer will wait in the middle of
+# a restart point on the standby.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_attach('CreateRestartPoint', 'wait');");
+
+# Execute a restart point on the standby, that we will now be waiting on.
+# This needs to be in the background.
+my $logstart = -s $node_standby->logfile;
+my $psql_session =
+ $node_standby->background_psql('postgres', on_error_stop => 0);
+$psql_session->query_until(
+ qr/starting_checkpoint/, q(
+ \echo starting_checkpoint
+ CHECKPOINT;
+));
+
+# Switch one WAL segment to make the previous restart point remove the
+# segment once the restart point completes.
+$node_primary->safe_psql('postgres', 'INSERT INTO prim_tab VALUES (1);');
+$node_primary->safe_psql('postgres', 'SELECT pg_switch_wal();');
+$node_primary->wait_for_replay_catchup($node_standby);
+
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
+
+# Check the logs that the restart point has started on standby. This is
+# optional, but let's be sure.
+ok( $node_standby->log_contains(
+ "restartpoint starting: immediate wait", $logstart),
+ "restartpoint has started");
+
+# Trigger promotion during the restart point.
+$node_primary->stop;
+$node_standby->promote;
+
+# Update the start position before waking up the checkpointer!
+$logstart = -s $node_standby->logfile;
+
+# Now wake up the checkpointer.
+$node_standby->safe_psql('postgres',
+ "SELECT injection_points_wakeup('CreateRestartPoint');");
+
+# Wait until the previous restart point completes on the newly-promoted
+# standby, checking the logs for that.
+my $checkpoint_complete = 0;
+foreach my $i (0 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
+{
+ if ($node_standby->log_contains("restartpoint complete"), $logstart)
+ {
+ $checkpoint_complete = 1;
+ last;
+ }
+ usleep(100_000);
+}
+is($checkpoint_complete, 1, 'restart point has completed');
+
+# Kill with SIGKILL, forcing all the backends to restart.
+my $psql_timeout = IPC::Run::timer(3600);
+my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', '');
+my $killme = IPC::Run::start(
+ [
+ 'psql', '-XAtq', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d',
+ $node_standby->connstr('postgres')
+ ],
+ '<',
+ \$killme_stdin,
+ '>',
+ \$killme_stdout,
+ '2>',
+ \$killme_stderr,
+ $psql_timeout);
+$killme_stdin .= q[
+SELECT pg_backend_pid();
+];
+$killme->pump until $killme_stdout =~ /[[:digit:]]+[\r\n]$/;
+my $pid = $killme_stdout;
+chomp($pid);
+$killme_stdout = '';
+$killme_stderr = '';
+
+my $ret = PostgreSQL::Test::Utils::system_log('pg_ctl', 'kill', 'KILL', $pid);
+is($ret, 0, 'killed process with KILL');
+
+# Wait until the server restarts, finish consuming output.
+$killme_stdin .= q[
+SELECT 1;
+];
+ok( pump_until(
+ $killme,
+ $psql_timeout,
+ \$killme_stderr,
+ qr/server closed the connection unexpectedly|connection to server was lost|could not send data to server/m
+ ),
+ "psql query died successfully after SIGKILL");
+$killme->finish;
+
+# Wait till server finishes restarting
+$node_standby->poll_query_until('postgres', undef, '');
+
+# After recovery, the server should be able to start.
+my $stdout;
+my $stderr;
+($ret, $stdout, $stderr) = $node_standby->psql('postgres', 'select 1');
+is($ret, 0, "psql connect success");
+is($stdout, 1, "psql select 1");
+
+done_testing();
--
2.43.0
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-26 08:24 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 00:55 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-26 08:24 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Mon, Feb 26, 2024 at 12:57:09PM +0900, Michael Paquier wrote:
> On Thu, Feb 22, 2024 at 08:00:24AM +0000, Bertrand Drouvot wrote:
> > +/* Maximum number of wait usable in injection points at once */
> >
> > s/Maximum number of wait/Maximum number of waits/ ?
>
> Thanks. I've edited a few more places while scanning the whole.
Thanks!
> >
> > 2 ===
> >
> > +# Check the logs that the restart point has started on standby. This is
> > +# optional, but let's be sure.
> > +my $log = slurp_file($node_standby->logfile, $logstart);
> > +my $checkpoint_start = 0;
> > +if ($log =~ m/restartpoint starting: immediate wait/)
> > +{
> > + $checkpoint_start = 1;
> > +}
> > +is($checkpoint_start, 1, 'restartpoint has started');
> >
> > what about?
> >
> > ok( $node_standby->log_contains( "restartpoint starting: immediate wait", $logstart),
> > "restartpoint has started");
>
> And I'm behind the commit that introduced it (392ea0c78fdb).
;-)
> It is
> possible to remove the dependency to slurp_file() entirely by
> switching the second location checking the logs for the checkpoint
> completion.
Yeah right.
> > Except for the above, v3 looks good to me.
>
> Thanks. I'm looking at applying that at the beginning of next week if
> there are no more comments, to get something by the feature freeze.
> We could be more flexible for all that as that's related to testing,
> but let's be in the clear.
Sounds reasonable to me.
> I've also renamed the test to 041_checkpoint_at_promote.pl, as now
> that the original is fixed, the checkpoint is not invalid. That's
> cleaner this way.
Agree.
I'll try to submit the POC patch in [1] before beginning of next week now that
we're "just waiting" if there is more comments on this current thread.
[1]: https://www.postgresql.org/message-id/ZdTNafYSxwnKNIhj%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 08:24 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-03-04 00:55 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 46+ messages in thread
From: Michael Paquier @ 2024-03-04 00:55 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, Feb 26, 2024 at 08:24:04AM +0000, Bertrand Drouvot wrote:
> I'll try to submit the POC patch in [1] before beginning of next week now that
> we're "just waiting" if there is more comments on this current thread.
I'll look at what you have there in more details.
> [1]: https://www.postgresql.org/message-id/ZdTNafYSxwnKNIhj%40ip-10-97-1-34.eu-west-3.compute.internal
The main routines have been now applied as 37b369dc67bc, with the test
in 6782709df81f. I have used the same naming policy as 6a1ea02c491d
for consistency, naming the injection point create-restart-point.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-26 09:10 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-26 09:10 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 26 Feb 2024, at 08:57, Michael Paquier <michael@paquier.xyz> wrote:
>
> <v4-0001-injection_points-Add-routines-to-wait-and-wake-pr.patch>
Would it be possible to have a helper function to check this:
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point”;
So that we could do something like
ok(node_standby->await_injection_point(“CreateRestartPoint”,”checkpointer"));
IMO, this could make many tests cleaner.
Or, perhaps, it’s a functionality for a future development?
Thanks!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-26 23:29 ` Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-26 23:29 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, Feb 26, 2024 at 02:10:49PM +0500, Andrey M. Borodin wrote:
> So that we could do something like
>
> ok(node_standby->await_injection_point(“CreateRestartPoint”,”checkpointer"));
It would be more flexible with a full string to describe the test
rather than a process name in the second argument.
> IMO, this could make many tests cleaner.
> Or, perhaps, it’s a functionality for a future development?
This could just happen as separate refactoring, I guess. But I'd wait
to see if more tests requiring scans to pg_stat_activity pop up. For
example, the test just posted here does not rely on that:
https://www.postgresql.org/message-id/ZdyZya4YrNapWKqz@ip-10-97-1-34.eu-west-3.compute.internal
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-27 06:00 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-27 06:00 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 27 Feb 2024, at 04:29, Michael Paquier <michael@paquier.xyz> wrote:
>
> For
> example, the test just posted here does not rely on that:
> https://www.postgresql.org/message-id/ZdyZya4YrNapWKqz@ip-10-97-1-34.eu-west-3.compute.internal
Instead, that test is scanning logs
+ # Note: $node_primary->wait_for_replay_catchup($node_standby) would be
+ # hanging here due to the injection point, so check the log instead.+
+ my $terminated = 0;
+ for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
+ {
+ if ($node_standby->log_contains(
+ 'terminating process .* to release replication slot \"injection_activeslot\"', $logstart))
+ {
+ $terminated = 1;
+ last;
+ }
+ usleep(100_000);
+ }
But, AFAICS, the purpose is the same: wait until event happened.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-27 11:08 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-27 11:08 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
>
>
> > On 27 Feb 2024, at 04:29, Michael Paquier <michael@paquier.xyz> wrote:
> >
> > For
> > example, the test just posted here does not rely on that:
> > https://www.postgresql.org/message-id/ZdyZya4YrNapWKqz@ip-10-97-1-34.eu-west-3.compute.internal
>
> Instead, that test is scanning logs
>
> + # Note: $node_primary->wait_for_replay_catchup($node_standby) would be
> + # hanging here due to the injection point, so check the log instead.+
> + my $terminated = 0;
> + for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
> + {
> + if ($node_standby->log_contains(
> + 'terminating process .* to release replication slot \"injection_activeslot\"', $logstart))
> + {
> + $terminated = 1;
> + last;
> + }
> + usleep(100_000);
> + }
>
> But, AFAICS, the purpose is the same: wait until event happened.
I think it's easier to understand the tests (I mean what the purpose of the
injection points are) if we don't use an helper function. While the helper
function would make the test easier to read / cleaner, I think it may make them
more difficult to understand as 'await_injection_point' would probably be too
generic.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-27 11:49 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-27 11:49 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
> On 27 Feb 2024, at 16:08, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
>
> On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
>>
>> But, AFAICS, the purpose is the same: wait until event happened.
>
> I think it's easier to understand the tests (I mean what the purpose of the
> injection points are) if we don't use an helper function. While the helper
> function would make the test easier to read / cleaner, I think it may make them
> more difficult to understand as 'await_injection_point' would probably be too
> generic.
For the record: I’m fine if there is no such function.
There will be at least one implementation of this function in every single test with waiting injection points.
That’s the case where we might want something generic.
What the specific there might be? The test can check that
- conditions are logged
- injection point reached in specific backend (e.g. checkpointer)
etc
I doubt that this specific checks worth cleanness of the test. And sacrificing test readability in favour of teaching reader what injection points are sounds strange.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-27 13:39 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-27 13:39 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Tue, Feb 27, 2024 at 04:49:03PM +0500, Andrey M. Borodin wrote:
> Hi,
>
> > On 27 Feb 2024, at 16:08, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
> >
> > On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
> >>
> >> But, AFAICS, the purpose is the same: wait until event happened.
> >
> > I think it's easier to understand the tests (I mean what the purpose of the
> > injection points are) if we don't use an helper function. While the helper
> > function would make the test easier to read / cleaner, I think it may make them
> > more difficult to understand as 'await_injection_point' would probably be too
> > generic.
>
> For the record: I’m fine if there is no such function.
> There will be at least one implementation of this function in every single test with waiting injection points.
> That’s the case where we might want something generic.
> What the specific there might be? The test can check that
> - conditions are logged
> - injection point reached in specific backend (e.g. checkpointer)
> etc
>
> I doubt that this specific checks worth cleanness of the test. And sacrificing test readability in favour of teaching reader what injection points are sounds strange.
I'm giving a second thought and it does not have to be exclusive, for example in
this specific test we could:
1) check that the injection point is reached with the helper (querying pg_stat_activity
looks good to me)
And
2) check in the log
So even if two checks might sound "too much" they both make sense to give 1) better
readability and 2) better understanding of the test.
So, I'm ok with the new helper too.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-28 04:26 ` Michael Paquier <michael@paquier.xyz>
2024-02-28 05:27 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 2 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-28 04:26 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Tue, Feb 27, 2024 at 01:39:59PM +0000, Bertrand Drouvot wrote:
> So, I'm ok with the new helper too.
If both of you feel strongly about that, I'm OK with introducing
something like that. Now, a routine should be only about waiting on
pg_stat_activity to report something, as for the logs we already have
log_contains(). And a test may want one check, but unlikely both.
Even if both are wanted, it's just a matter of using log_contains()
and the new routine that does pg_stat_activity lookups.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-28 05:27 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 05:54 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-28 05:27 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 28 Feb 2024, at 09:26, Michael Paquier <michael@paquier.xyz> wrote:
>
> Now, a routine should be only about waiting on
> pg_stat_activity to report something
BTW, if we had an SQL function such as injection_point_await(name), we could use it in our isolation tests as well as our TAP tests :)
However, this might well be a future improvement, if we had a generic “await" Perl function - we wouldn’t need to re-use new SQL code in so many places.
Thanks!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 05:27 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-29 05:54 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-29 05:54 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, Feb 28, 2024 at 10:27:52AM +0500, Andrey M. Borodin wrote:
> BTW, if we had an SQL function such as injection_point_await(name),
> we could use it in our isolation tests as well as our TAP tests :)
I am not quite sure to follow here. The isolation test facility now
relies on the in-core function pg_isolation_test_session_is_blocked()
to check the state of backends getting blocked, and that's outside of
the scope of the module injection_points.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-28 06:20 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-28 06:20 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
> On Tue, Feb 27, 2024 at 01:39:59PM +0000, Bertrand Drouvot wrote:
> > So, I'm ok with the new helper too.
>
> If both of you feel strongly about that, I'm OK with introducing
> something like that.
Thanks!
> Now, a routine should be only about waiting on
> pg_stat_activity to report something, as for the logs we already have
> log_contains().
Agree.
> And a test may want one check, but unlikely both.
> Even if both are wanted, it's just a matter of using log_contains()
> and the new routine that does pg_stat_activity lookups.
Yeah, that's also my point of view.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-29 05:56 ` Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-29 05:56 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, Feb 28, 2024 at 06:20:41AM +0000, Bertrand Drouvot wrote:
> On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
>> On Tue, Feb 27, 2024 at 01:39:59PM +0000, Bertrand Drouvot wrote:
>> > So, I'm ok with the new helper too.
>>
>> If both of you feel strongly about that, I'm OK with introducing
>> something like that.
>
> Thanks!
(Cough. As in, "feel free to send a patch" on top of what's already
proposed if any of you feel that's better at the end.)
;)
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-29 08:35 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-29 08:35 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Thu, Feb 29, 2024 at 02:56:23PM +0900, Michael Paquier wrote:
> On Wed, Feb 28, 2024 at 06:20:41AM +0000, Bertrand Drouvot wrote:
> > On Wed, Feb 28, 2024 at 01:26:46PM +0900, Michael Paquier wrote:
> >> On Tue, Feb 27, 2024 at 01:39:59PM +0000, Bertrand Drouvot wrote:
> >> > So, I'm ok with the new helper too.
> >>
> >> If both of you feel strongly about that, I'm OK with introducing
> >> something like that.
> >
> > Thanks!
>
> (Cough. As in, "feel free to send a patch" on top of what's already
> proposed if any of you feel that's better at the end.)
>
> ;)
okay ;-)
Something like the attached?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v1-0001-Adding-wait_for_injection_point-helper.patch (text/x-diff, 2.4K) download
From 6593970648c6cff3150e602746bf6cadffb728cc Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 29 Feb 2024 08:31:28 +0000
Subject: [PATCH v1] Adding wait_for_injection_point helper
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 24 +++++++++++++++++++
.../recovery/t/041_checkpoint_at_promote.pl | 8 +------
2 files changed, 25 insertions(+), 7 deletions(-)
57.1% src/test/perl/PostgreSQL/Test/
42.8% src/test/recovery/t/
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 44c1bb5afd..414b4ef421 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2742,6 +2742,30 @@ sub lsn
=pod
+=item $node->wait_for_injection_point(injection_name, backend_type)
+
+Wait for the backend_type to wait for the injection point name.
+
+=cut
+
+sub wait_for_injection_point
+{
+ my ($self, $injection_name, $backend_type) = @_;
+
+ $self->poll_query_until(
+ 'postgres', qq[
+ SELECT count(*) > 0 FROM pg_stat_activity
+ WHERE backend_type = '$backend_type' AND wait_event = '$injection_name'
+ ])
+ or die
+ "timed out waiting for the backend type to wait for the injection point name";
+
+ print "done\n";
+ return;
+}
+
+=pod
+
=item $node->wait_for_catchup(standby_name, mode, target_lsn)
Wait for the replication connection with application_name standby_name until
diff --git a/src/test/recovery/t/041_checkpoint_at_promote.pl b/src/test/recovery/t/041_checkpoint_at_promote.pl
index 47381a2c82..3d6faabc0b 100644
--- a/src/test/recovery/t/041_checkpoint_at_promote.pl
+++ b/src/test/recovery/t/041_checkpoint_at_promote.pl
@@ -79,13 +79,7 @@ $node_primary->wait_for_replay_catchup($node_standby);
# Wait until the checkpointer is in the middle of the restart point
# processing, relying on the custom wait event generated in the
# wait callback used in the injection point previously attached.
-ok( $node_standby->poll_query_until(
- 'postgres',
- qq[SELECT count(*) FROM pg_stat_activity
- WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
- '1'),
- 'checkpointer is waiting in restart point'
-) or die "Timed out while waiting for checkpointer to run restart point";
+$node_standby->wait_for_injection_point('CreateRestartPoint','checkpointer');
# Check the logs that the restart point has started on standby. This is
# optional, but let's be sure.
--
2.34.1
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-29 09:34 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-29 09:34 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 29 Feb 2024, at 13:35, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
>
> Something like the attached?
There's extraneous print "done\n";
Also can we have optional backend type :)
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-29 10:20 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-29 10:20 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Thu, Feb 29, 2024 at 02:34:35PM +0500, Andrey M. Borodin wrote:
>
>
> > On 29 Feb 2024, at 13:35, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Something like the attached?
>
> There's extraneous print "done\n";
doh! bad copy/paste ;-)
> Also can we have optional backend type :)
done in v2 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v2-0001-Adding-wait_for_injection_point-helper.patch (text/x-diff, 2.5K) download
From a2ece828e93e6e919243e8d2529b33d48d05f084 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 29 Feb 2024 08:31:28 +0000
Subject: [PATCH v2] Adding wait_for_injection_point helper
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 25 +++++++++++++++++++
.../recovery/t/041_checkpoint_at_promote.pl | 8 +-----
2 files changed, 26 insertions(+), 7 deletions(-)
59.8% src/test/perl/PostgreSQL/Test/
40.1% src/test/recovery/t/
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 44c1bb5afd..559d3febca 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2742,6 +2742,31 @@ sub lsn
=pod
+=item $node->wait_for_injection_point(injection_name, backend_type)
+
+Wait for the backend_type to wait for the injection point name.
+
+=cut
+
+sub wait_for_injection_point
+{
+ my ($self, $injection_name, $backend_type) = @_;
+
+ $backend_type = defined($backend_type) ? qq('$backend_type') : 'backend_type';
+
+ $self->poll_query_until(
+ 'postgres', qq[
+ SELECT count(*) > 0 FROM pg_stat_activity
+ WHERE backend_type = $backend_type AND wait_event = '$injection_name'
+ ])
+ or die
+ "timed out waiting for the backend type to wait for the injection point name";
+
+ return;
+}
+
+=pod
+
=item $node->wait_for_catchup(standby_name, mode, target_lsn)
Wait for the replication connection with application_name standby_name until
diff --git a/src/test/recovery/t/041_checkpoint_at_promote.pl b/src/test/recovery/t/041_checkpoint_at_promote.pl
index 47381a2c82..3d6faabc0b 100644
--- a/src/test/recovery/t/041_checkpoint_at_promote.pl
+++ b/src/test/recovery/t/041_checkpoint_at_promote.pl
@@ -79,13 +79,7 @@ $node_primary->wait_for_replay_catchup($node_standby);
# Wait until the checkpointer is in the middle of the restart point
# processing, relying on the custom wait event generated in the
# wait callback used in the injection point previously attached.
-ok( $node_standby->poll_query_until(
- 'postgres',
- qq[SELECT count(*) FROM pg_stat_activity
- WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
- '1'),
- 'checkpointer is waiting in restart point'
-) or die "Timed out while waiting for checkpointer to run restart point";
+$node_standby->wait_for_injection_point('CreateRestartPoint','checkpointer');
# Check the logs that the restart point has started on standby. This is
# optional, but let's be sure.
--
2.34.1
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-02-29 13:19 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-29 13:19 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Michael Paquier <michael@paquier.xyz>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 29 Feb 2024, at 15:20, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
>
> done in v2 attached.
Works fine for me. Thanks!
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-03-01 02:02 ` Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-03-01 02:02 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Thu, Feb 29, 2024 at 06:19:58PM +0500, Andrey M. Borodin wrote:
> Works fine for me. Thanks!
+ "timed out waiting for the backend type to wait for the injection point name";
The log should provide some context about the caller failing, meaning
that the backend type and the injection point name should be mentioned
in these logs to help in debugging issues.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-01 06:52 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Bertrand Drouvot @ 2024-03-01 06:52 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Fri, Mar 01, 2024 at 11:02:01AM +0900, Michael Paquier wrote:
> On Thu, Feb 29, 2024 at 06:19:58PM +0500, Andrey M. Borodin wrote:
> > Works fine for me. Thanks!
>
> + "timed out waiting for the backend type to wait for the injection point name";
>
> The log should provide some context about the caller failing, meaning
> that the backend type and the injection point name should be mentioned
> in these logs to help in debugging issues.
Yeah, done in v3 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
v3-0001-Adding-wait_for_injection_point-helper.patch (text/x-diff, 2.6K) download
From ea354b9d0aeee34b9ba315735cce1e2ff1cb6532 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Thu, 29 Feb 2024 08:31:28 +0000
Subject: [PATCH v3] Adding wait_for_injection_point helper
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 36 +++++++++++++++++++
.../recovery/t/041_checkpoint_at_promote.pl | 8 +----
2 files changed, 37 insertions(+), 7 deletions(-)
65.3% src/test/perl/PostgreSQL/Test/
34.6% src/test/recovery/t/
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 44c1bb5afd..8d92eb3858 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2742,6 +2742,42 @@ sub lsn
=pod
+=item $node->wait_for_injection_point(injection_name, backend_type)
+
+Wait for the backend_type to wait for the injection point name.
+
+=cut
+
+sub wait_for_injection_point
+{
+ my ($self, $injection_name, $backend_type) = @_;
+ my $die_message;
+
+ if (defined($backend_type))
+ {
+ $backend_type = qq('$backend_type');
+ $die_message = "the backend type $backend_type";
+ }
+ else
+ {
+ $backend_type = 'backend_type';
+ $die_message = 'one backend';
+
+ }
+
+ $self->poll_query_until(
+ 'postgres', qq[
+ SELECT count(*) > 0 FROM pg_stat_activity
+ WHERE backend_type = $backend_type AND wait_event = '$injection_name'
+ ])
+ or die
+ qq(timed out waiting for $die_message to wait for the injection point '$injection_name');
+
+ return;
+}
+
+=pod
+
=item $node->wait_for_catchup(standby_name, mode, target_lsn)
Wait for the replication connection with application_name standby_name until
diff --git a/src/test/recovery/t/041_checkpoint_at_promote.pl b/src/test/recovery/t/041_checkpoint_at_promote.pl
index 47381a2c82..3d6faabc0b 100644
--- a/src/test/recovery/t/041_checkpoint_at_promote.pl
+++ b/src/test/recovery/t/041_checkpoint_at_promote.pl
@@ -79,13 +79,7 @@ $node_primary->wait_for_replay_catchup($node_standby);
# Wait until the checkpointer is in the middle of the restart point
# processing, relying on the custom wait event generated in the
# wait callback used in the injection point previously attached.
-ok( $node_standby->poll_query_until(
- 'postgres',
- qq[SELECT count(*) FROM pg_stat_activity
- WHERE backend_type = 'checkpointer' AND wait_event = 'CreateRestartPoint' ;],
- '1'),
- 'checkpointer is waiting in restart point'
-) or die "Timed out while waiting for checkpointer to run restart point";
+$node_standby->wait_for_injection_point('CreateRestartPoint','checkpointer');
# Check the logs that the restart point has started on standby. This is
# optional, but let's be sure.
--
2.34.1
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
@ 2024-03-04 01:44 ` Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 07:22 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
0 siblings, 2 replies; 46+ messages in thread
From: Michael Paquier @ 2024-03-04 01:44 UTC (permalink / raw)
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Fri, Mar 01, 2024 at 06:52:45AM +0000, Bertrand Drouvot wrote:
> + if (defined($backend_type))
> + {
> + $backend_type = qq('$backend_type');
> + $die_message = "the backend type $backend_type";
> + }
> + else
> + {
> + $backend_type = 'backend_type';
> + $die_message = 'one backend';
> +
> + }
> +
> + $self->poll_query_until(
> + 'postgres', qq[
> + SELECT count(*) > 0 FROM pg_stat_activity
> + WHERE backend_type = $backend_type AND wait_event = '$injection_name'
> + ])
> + or die
> + qq(timed out waiting for $die_message to wait for the injection point '$injection_name');
I was looking at that, and found v3 to be an overkill. First, I think
that we should encourage callers to pass down a backend_type. Perhaps
I am wrong to assume so, but that's based on my catalog of tests
waiting in my stack.
A second thing is that this is entirely unrelated to injection points,
because a test may want to wait for a given wait_event on a
backend_type without using the module injection_points. At the end, I
have renamed the routine to wait_for_event(), tweaked a bit its
internals, and the result looked fine so I have applied it while
updating 041_checkpoint_at_promote.pl to use it. Its internals could
always be expanded more depending on the demand for it.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-04 04:17 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:16 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 2 replies; 46+ messages in thread
From: Jelte Fennema-Nio @ 2024-03-04 04:17 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
I noticed this was committed, and took a quick look. It sounds very
useful for testing Citus to be able to use injection points too, but I
noticed this code is included in src/test/modules. It sounds like that
location will make it somewhat hard to install. If that's indeed the
case, would it be possible to move it to contrib instead?
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
@ 2024-03-04 05:16 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
1 sibling, 0 replies; 46+ messages in thread
From: Andrey M. Borodin @ 2024-03-04 05:16 UTC (permalink / raw)
To: Jelte Fennema-Nio <postgres@jeltef.nl>; +Cc: Michael Paquier <michael@paquier.xyz>; Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
> On 4 Mar 2024, at 06:44, Michael Paquier <michael@paquier.xyz> wrote:
> so I have applied it
Great! Thank you! A really useful stuff for an asynchronous testing!
> On 4 Mar 2024, at 09:17, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> this code is included in src/test/modules. It sounds like that
> location will make it somewhat hard to install.
+1. I joined the thread too late to ask why it’s not in core. But, it seems to me that separating logic even further is not necessary, given build option —with-injection-points off by default.
> If that's indeed the
> case, would it be possible to move it to contrib instead?
There’s no point in shipping this to users, especially with the build without injection points compiled.
Best regards, Andrey Borodin.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
@ 2024-03-04 05:27 ` Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
1 sibling, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-03-04 05:27 UTC (permalink / raw)
To: Jelte Fennema-Nio <postgres@jeltef.nl>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, Mar 04, 2024 at 05:17:52AM +0100, Jelte Fennema-Nio wrote:
> I noticed this was committed, and took a quick look. It sounds very
> useful for testing Citus to be able to use injection points too, but I
> noticed this code is included in src/test/modules. It sounds like that
> location will make it somewhat hard to install. If that's indeed the
> case, would it be possible to move it to contrib instead?
One problem with installing that in contrib/ is that it would require
more maintenance as a stable and "released" module. The aim of this
module is to be more flexible than that, so as it is possible to
extend it at will even in the back branches to be able to implement
features that could help with tests that we'd want to implement in
stable branches. I have mentioned that on a separate thread, but
adding more extension maintenance burden while implementing complex
tests does not sound like a good idea for me in the long-run.
Perhaps we could consider that as an exception in "contrib", or have a
separate path for test modules we're OK to install (the calls had
better be superuser-only if we do that). Another thing with the
backend support of injection points is that you could implement your
own extension within citus, able to do what you mimic this in-core
module, and get inspiration from it. Duplication is never cool, I
agree, though.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-04 09:51 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Jelte Fennema-Nio @ 2024-03-04 09:51 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, 4 Mar 2024 at 06:27, Michael Paquier <michael@paquier.xyz> wrote:
> I have mentioned that on a separate thread,
Yeah, I didn't read all emails related to this feature
> Perhaps we could consider that as an exception in "contrib", or have a
> separate path for test modules we're OK to install (the calls had
> better be superuser-only if we do that).
Yeah, it makes sense that you'd want to backport fixes/changes to
this. As long as you put a disclaimer in the docs that you can do that
for this module, I think it would be fine. Our tests fairly regularly
break anyway when changing minor versions of postgres in our CI, e.g.
due to improvements in the output of isolationtester. So if changes to
this module require some changes that's fine by me. Seems much nicer
than having to copy-paste the code.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
@ 2024-03-04 22:23 ` Michael Paquier <michael@paquier.xyz>
2024-03-05 08:43 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-03-04 22:23 UTC (permalink / raw)
To: Jelte Fennema-Nio <postgres@jeltef.nl>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, Mar 04, 2024 at 10:51:41AM +0100, Jelte Fennema-Nio wrote:
> Yeah, it makes sense that you'd want to backport fixes/changes to
> this. As long as you put a disclaimer in the docs that you can do that
> for this module, I think it would be fine. Our tests fairly regularly
> break anyway when changing minor versions of postgres in our CI, e.g.
> due to improvements in the output of isolationtester. So if changes to
> this module require some changes that's fine by me. Seems much nicer
> than having to copy-paste the code.
In my experience, anybody who does serious testing with their product
integrated with Postgres have two or three types of builds with their
own scripts: one with assertions, -DG and other developer-oriented
options enabled, and one for production deployments with more
optimized options like -O2. Once there are custom scripts to build
and package Postgres, do we really need to move that to contrib/ at
all? make install would work for a test module as long as the command
is run locally in its directory.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-05 08:43 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 06:17 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Jelte Fennema-Nio @ 2024-03-05 08:43 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Mon, 4 Mar 2024 at 23:23, Michael Paquier <michael@paquier.xyz> wrote:
> In my experience, anybody who does serious testing with their product
> integrated with Postgres have two or three types of builds with their
> own scripts: one with assertions, -DG and other developer-oriented
> options enabled, and one for production deployments with more
> optimized options like -O2. Once there are custom scripts to build
> and package Postgres, do we really need to move that to contrib/ at
> all?
I do think there is quite a bit of a difference from a user
perspective to providing a few custom configure flags and having to go
to a separate directory and run "make install" there too. Simply
because it's yet another step. For dev environments most/all of my
team uses pgenv: https://github.com/theory/pgenv There I agree we
could add functionality to it to also install test modules when given
a certain flag/environment variable, but it would be nice if that
wasn't needed.
One big downside to not having it in contrib is that we also run tests
of Citus against official PGDG postgres packages and those would
likely not include these test modules, so we wouldn't be able to run
all our tests then.
Also I think the injection points extension is quite different from
the other modules in src/modules/test. All of these other modules are
basically test binaries that need to be separate modules, but they
don't provide any useful functionality when installing them. The
injection_poinst one actually provides useful functionality itself,
that can be used by other people testing things against postgres.
> make install would work for a test module as long as the command
> is run locally in its directory.
What would I need to do for meson builds? I tried the following
command but it doesn't seem to actually install the injection points
command:
ninja -C build install-test-files
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-05 08:43 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
@ 2024-03-06 06:17 ` Michael Paquier <michael@paquier.xyz>
2024-03-06 09:19 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-03-06 06:17 UTC (permalink / raw)
To: Jelte Fennema-Nio <postgres@jeltef.nl>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Tue, Mar 05, 2024 at 09:43:03AM +0100, Jelte Fennema-Nio wrote:
> I do think there is quite a bit of a difference from a user
> perspective to providing a few custom configure flags and having to go
> to a separate directory and run "make install" there too. Simply
> because it's yet another step. For dev environments most/all of my
> team uses pgenv: https://github.com/theory/pgenv There I agree we
> could add functionality to it to also install test modules when given
> a certain flag/environment variable, but it would be nice if that
> wasn't needed.
Didn't know this one, TBH. That's interesting. My own scripts
emulate the same things with versioning, patching, etc.
> One big downside to not having it in contrib is that we also run tests
> of Citus against official PGDG postgres packages and those would
> likely not include these test modules, so we wouldn't be able to run
> all our tests then.
No idea about this part, unfortunately. One thing that I'd do is
first check if the in-core module is enough to satisfy the
requirements Citus would want. I got pinged about Timescale and
greenplum recently, and they can reuse the backends APIs, but they've
also wanted a few more callbacks than the in-core module so they will
need to have their own code for tests with a custom callback library.
Perhaps we could move that to contrib/ and document that this is a
module for testing, that can be updated without notice even in minor
upgrades and that there are no version upgrades done, or something
like that. I'm open to that if there's enough demand for it, but I
don't know how much we should accomodate with the existing
requirements of contrib/ for something that's only developer-oriented.
> Also I think the injection points extension is quite different from
> the other modules in src/modules/test. All of these other modules are
> basically test binaries that need to be separate modules, but they
> don't provide any useful functionality when installing them. The
> injection_poinst one actually provides useful functionality itself,
> that can be used by other people testing things against postgres.
I'm not sure about that yet, TBH. Anything that gets added to this
module should be used in some way by the in-core tests, or just not be
there. That's a line I don't want to cross, which is why it's a test
module. FWIW, it would be really annoying to have documentation
requirements, actually, because that increases maintenance and I'm not
sure it's a good idea to add a module maintenance on top of what could
require more facility in the module to implement a test for a bug fix.
> What would I need to do for meson builds? I tried the following
> command but it doesn't seem to actually install the injection points
> command:
> ninja -C build install-test-files
Weird, that works here. I was digging into the meson tree and noticed
that this was the only run_target() related to the test module install
data, and injection_points gets installed as well as all the other
modules. This should just need -Dinjection_points=true.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-05 08:43 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 06:17 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-06 09:19 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 22:54 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Jelte Fennema-Nio @ 2024-03-06 09:19 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, 6 Mar 2024 at 07:17, Michael Paquier <michael@paquier.xyz> wrote:
> I'm open to that if there's enough demand for it, but I
> don't know how much we should accomodate with the existing
> requirements of contrib/ for something that's only developer-oriented.
There's quite a few developer-oriented GUCs as well, even requiring
their own DEVELOPER_OPTIONS section. So I don't think it's too weird
to have some developer-oriented extensions too.
> I'm not sure about that yet, TBH. Anything that gets added to this
> module should be used in some way by the in-core tests, or just not be
> there. That's a line I don't want to cross, which is why it's a test
> module.
That seems fine, if people need more functionality they can indeed
create their own test helpers. It mainly seems annoying for people to
have to copy-paste the ones from core to their own extension.
What I mainly meant is that anything in src/test/modules is not even
close to somewhat useful for other people to use. They are really just
very specific tests that need to be written in C. Afaict all those
modules are not even used by tests outside of their own module. But
these functions are helper functions, to be used by other tests. And
limiting the users of those helper functions to just be in-core
Postgres code seems a bit annoying. I feel like these functions are
more akin to the pgregress/isolationtester binaries in their usage,
than akin to other modules in src/test/modules.
> FWIW, it would be really annoying to have documentation
> requirements, actually, because that increases maintenance and I'm not
> sure it's a good idea to add a module maintenance on top of what could
> require more facility in the module to implement a test for a bug fix.
Quite a few contrib modules have very limited documentation. I think
it would be fine for this as well.
> This should just need -Dinjection_points=true.
Ugh... Sorry... I didn't realize that it needed a dedicated configure
flag. When providing that flag it indeed installs the expected files.
I guess that rules out testing against PGDG packages, because those
packages almost certainly wouldn't specify this flag.
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:27 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-05 08:43 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 06:17 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-06 09:19 ` Re: Injection points: some tools to wait and wake Jelte Fennema-Nio <postgres@jeltef.nl>
@ 2024-03-06 22:54 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 46+ messages in thread
From: Michael Paquier @ 2024-03-06 22:54 UTC (permalink / raw)
To: Jelte Fennema-Nio <postgres@jeltef.nl>; +Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
On Wed, Mar 06, 2024 at 10:19:41AM +0100, Jelte Fennema-Nio wrote:
> What I mainly meant is that anything in src/test/modules is not even
> close to somewhat useful for other people to use. They are really just
> very specific tests that need to be written in C. Afaict all those
> modules are not even used by tests outside of their own module. But
> these functions are helper functions, to be used by other tests. And
> limiting the users of those helper functions to just be in-core
> Postgres code seems a bit annoying. I feel like these functions are
> more akin to the pgregress/isolationtester binaries in their usage,
> than akin to other modules in src/test/modules.
Perhaps. I think that we're still in the discovery phase for this
stuff, and more people should get used to it first (this will take
some time and everybody is busy with their own stuff for the last
commit fest). At least it does not seem good to rush any decisions at
this stage.
>> FWIW, it would be really annoying to have documentation
>> requirements, actually, because that increases maintenance and I'm not
>> sure it's a good idea to add a module maintenance on top of what could
>> require more facility in the module to implement a test for a bug fix.
>
> Quite a few contrib modules have very limited documentation. I think
> it would be fine for this as well.
I'd argue that we should try to improve the existing documentation
rather that use that as an argument to add more modules with limited
documentation ;)
> Ugh... Sorry... I didn't realize that it needed a dedicated configure
> flag. When providing that flag it indeed installs the expected files.
> I guess that rules out testing against PGDG packages, because those
> packages almost certainly wouldn't specify this flag.
The CI enables the switch, and I've updated all my buildfarm members
to use it. In terms of coverage, that's already quite good.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-03-04 07:22 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
1 sibling, 0 replies; 46+ messages in thread
From: Bertrand Drouvot @ 2024-03-04 07:22 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Mon, Mar 04, 2024 at 10:44:34AM +0900, Michael Paquier wrote:
> On Fri, Mar 01, 2024 at 06:52:45AM +0000, Bertrand Drouvot wrote:
> > + if (defined($backend_type))
> > + {
> > + $backend_type = qq('$backend_type');
> > + $die_message = "the backend type $backend_type";
> > + }
> > + else
> > + {
> > + $backend_type = 'backend_type';
> > + $die_message = 'one backend';
> > +
> > + }
> > +
> > + $self->poll_query_until(
> > + 'postgres', qq[
> > + SELECT count(*) > 0 FROM pg_stat_activity
> > + WHERE backend_type = $backend_type AND wait_event = '$injection_name'
> > + ])
> > + or die
> > + qq(timed out waiting for $die_message to wait for the injection point '$injection_name');
>
> I was looking at that, and found v3 to be an overkill. First, I think
> that we should encourage callers to pass down a backend_type. Perhaps
> I am wrong to assume so, but that's based on my catalog of tests
> waiting in my stack.
Works for me.
> A second thing is that this is entirely unrelated to injection points,
> because a test may want to wait for a given wait_event on a
> backend_type without using the module injection_points. At the end, I
> have renamed the routine to wait_for_event(),
Good idea, fully makes sense.
> tweaked a bit its
> internals, and the result looked fine so I have applied it
Thanks!
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Re: Injection points: some tools to wait and wake Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-21 08:07 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
1 sibling, 0 replies; 46+ messages in thread
From: Bertrand Drouvot @ 2024-02-21 08:07 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Hi,
On Wed, Feb 21, 2024 at 07:08:03AM +0900, Michael Paquier wrote:
> On Tue, Feb 20, 2024 at 03:55:08PM +0000, Bertrand Drouvot wrote:
> > +PG_FUNCTION_INFO_V1(injection_points_wake);
> > +Datum
> > +injection_points_wake(PG_FUNCTION_ARGS)
> > +{
> >
> > I think that This function will wake up all the "wait" injection points.
> > Would that make sense to implement some filtering based on the name? That could
> > be useful for tests that would need multiple wait injection points and that want
> > to wake them up "sequentially".
> >
> > We could think about it if there is such a need in the future though.
>
> Well, both you and Andrey are asking for it now, so let's do it.
Thanks!
> The implementation is simple:
> - Store in InjectionPointSharedState an array of wait_counts and an
> array of names. There is only one condition variable.
> - When a point wants to wait, it takes the spinlock and looks within
> the array of names until it finds a free slot, adds its name into the
> array to reserve a wait counter at the same position, releases the
> spinlock. Then it loops on the condition variable for an update of
> the counter it has reserved. It is possible to make something more
> efficient, but at a small size it would not really matter.
> - The wakeup takes a point name in argument, acquires the spinlock,
> and checks if it can find the point into the array, pinpoints the
> location of the counter to update and updates it. Then it broadcasts
> the change.
> - The wait loop checks its counter, leaves its loop, cancels the
> sleep, takes the spinlock to unregister from the array, and leaves.
>
I think that makes sense and now the "counter" makes more sense to me (thanks to
it we don't need multiple CV).
> I would just hardcode the number of points that can wait, say 5 of
> them tracked in shmem? Does that look like what you are looking at?
I think so yes and more than 5 points would look like a complicated test IMHO.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-19 08:54 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-19 23:21 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-19 08:54 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat@gmail.com>
> On 19 Feb 2024, at 09:01, Michael Paquier <michael@paquier.xyz> wrote:
>
> Thoughts and comments are welcome.
Hi Michael,
thanks for your work on injection points! I want to test a bunch of stuff using this facility.
I have a wishlist of functionality that I'd like to see in injection points. I hope you will find some of these ideas useful to improve the feature.
1. injection_points_wake() will wake all of waiters. But it's not suitable for complex tests. I think there must be a way to wake only specific waiter by injection point name.
2. Alexander Korotkov's stopevents could be used in isolation tests. This kind of tests is perfect for describing complex race conditions. (as a side note, I'd be happy if we could have primary\standby in isolation tests too)
3. Can we have some Perl function for this?
+# Wait until the checkpointer is in the middle of the restart point
+# processing, relying on the custom wait event generated in the
+# wait callback used in the injection point previously attached.
+ok( $node_standby->poll_query_until(
+ 'postgres',
+ qq[SELECT count(*) FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' AND wait_event = 'injection_wait' ;],
+ '1'),
+ 'checkpointer is waiting in restart point'
+) or die "Timed out while waiting for checkpointer to run restart point";
Perhaps something like
$node->do_a_query_and_wait_for_injection_point_observed(sql,injection_point_name);
4. Maybe I missed it, but I'd like to see a guideline on how to name injection points.
5. In many cases we need to have injection point under critical section. I propose to have a "prepared injection point". See [0] for example in v2-0003-Test-multixact-CV-sleep.patch
+ INJECTION_POINT_PREPARE("GetNewMultiXactId-done");
+
START_CRIT_SECTION();
+ INJECTION_POINT_RUN_PREPARED();
6. Currently our codebase have files injection_point.c and injection_points.c. It's very difficult to remember which is where...
7. This is extremely distant, but some DBMSs allow to enable injection points by placing files on the filesystem. That would allow to test something during recovery when no SQL interface is present.
Let's test all the neat stuff! Thank you!
Best regards, Andrey Borodin.
[0] https://www.postgresql.org/message-id/0925F9A9-4D53-4B27-A87E-3D83A757B0E0@yandex-team.ru
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 08:54 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-19 23:21 ` Michael Paquier <michael@paquier.xyz>
2024-02-20 14:32 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
0 siblings, 1 reply; 46+ messages in thread
From: Michael Paquier @ 2024-02-19 23:21 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat@gmail.com>
On Mon, Feb 19, 2024 at 11:54:20AM +0300, Andrey M. Borodin wrote:
> 1. injection_points_wake() will wake all of waiters. But it's not
> suitable for complex tests. I think there must be a way to wake only
> specific waiter by injection point name.
I don't disagree with that, but I don't have a strong argument for
implementing that until there is an explicit need for it in core. It
is also possible to plug in your own module, outside of core, if you
are looking for something more specific. The backend APIs allow that.
> 2. Alexander Korotkov's stopevents could be used in isolation
> tests. This kind of tests is perfect for describing complex race
> conditions. (as a side note, I'd be happy if we could have
> primary\standby in isolation tests too)
This requires plugging is more into src/test/isolation/, with multiple
connection strings. This has been suggested in the past.
> 3. Can we have some Perl function for this?
> +# Wait until the checkpointer is in the middle of the restart point
> +# processing, relying on the custom wait event generated in the
> +# wait callback used in the injection point previously attached.
> +ok( $node_standby->poll_query_until(
> + 'postgres',
> + qq[SELECT count(*) FROM pg_stat_activity
> + WHERE backend_type = 'checkpointer' AND wait_event = 'injection_wait' ;],
> + '1'),
> + 'checkpointer is waiting in restart point'
> +) or die "Timed out while waiting for checkpointer to run restart point";
>
> Perhaps something like
> $node->do_a_query_and_wait_for_injection_point_observed(sql,injection_point_name);
I don't see why not. But I'm not sure how much I need to plug in into
the main modules yet.
> 4. Maybe I missed it, but I'd like to see a guideline on how to name
> injection points.
I don't think we have any of that, or even that we need one. In
short, I'm OK to be more consistent with the choice of ginbtree.c and
give priority to it and make it more official in the docs.
> 5. In many cases we need to have injection point under critical
> section. I propose to have a "prepared injection point". See [0] for
> example in v2-0003-Test-multixact-CV-sleep.patch
> + INJECTION_POINT_PREPARE("GetNewMultiXactId-done");
> +
> START_CRIT_SECTION();
>
> + INJECTION_POINT_RUN_PREPARED();
I don't see how that's different from a wait/wake logic? The only
thing you've changed is to stop a wait when a point is detached and
you want to make the stop conditional. Plugging in a condition
variable is more flexible than a hardcoded sleep in terms of wait,
while being more responsive.
> 7. This is extremely distant, but some DBMSs allow to enable
> injection points by placing files on the filesystem. That would
> allow to test something during recovery when no SQL interface is
> present.
Yeah, I could see why you'd want to do something like that. If a use
case pops up, that can surely be discussed.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 08:54 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-19 23:21 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
@ 2024-02-20 14:32 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-20 23:22 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 46+ messages in thread
From: Andrey M. Borodin @ 2024-02-20 14:32 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat@gmail.com>
> On 20 Feb 2024, at 02:21, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Feb 19, 2024 at 11:54:20AM +0300, Andrey M. Borodin wrote:
>> 1. injection_points_wake() will wake all of waiters. But it's not
>> suitable for complex tests. I think there must be a way to wake only
>> specific waiter by injection point name.
>
> I don't disagree with that, but I don't have a strong argument for
> implementing that until there is an explicit need for it in core. It
> is also possible to plug in your own module, outside of core, if you
> are looking for something more specific. The backend APIs allow that.
In [0] I want to create a test for edge case of reading recent multixact. External module does not allow to have a test within repository.
In that test I need to sync backends in 3 steps (backend 1 starts to wait in injection point, backend 2 starts to sleep in other point, backend 1 is released and observe 3rd injection point). "wake them all" implementation allows only 2-step synchronization.
I will try to simplify test to 2-step, but it would be much easier to implement if injection points could be awaken independently.
>> 2. Alexander Korotkov's stopevents could be used in isolation
>> tests. This kind of tests is perfect for describing complex race
>> conditions. (as a side note, I'd be happy if we could have
>> primary\standby in isolation tests too)
>
> This requires plugging is more into src/test/isolation/, with multiple
> connection strings. This has been suggested in the past.
I think standby isolation tests are just a sugar-on-top feature here.
Wrt injection points, I'd like to see a function to wait until some injection point is observed.
With this function at hand developer can implement race condition tests as an isolation test.
>> 5. In many cases we need to have injection point under critical
>> section. I propose to have a "prepared injection point". See [0] for
>> example in v2-0003-Test-multixact-CV-sleep.patch
>> + INJECTION_POINT_PREPARE("GetNewMultiXactId-done");
>> +
>> START_CRIT_SECTION();
>>
>> + INJECTION_POINT_RUN_PREPARED();
>
> I don't see how that's different from a wait/wake logic? The only
> thing you've changed is to stop a wait when a point is detached and
> you want to make the stop conditional. Plugging in a condition
> variable is more flexible than a hardcoded sleep in terms of wait,
> while being more responsive.
No, "prepared injection point" is not about wait\wake logic. It's about having injection point in critical section.
Normal injection point will pstrdup(name) and fail. In [0] I need a test that waits after multixact generation before WAL-logging it. It's only possible in a critical section.
Thanks!
Best regards, Andrey Borodin.
[0] https://www.postgresql.org/message-id/0925F9A9-4D53-4B27-A87E-3D83A757B0E0@yandex-team.ru
^ permalink raw reply [nested|flat] 46+ messages in thread
* Re: Injection points: some tools to wait and wake
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 08:54 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-19 23:21 ` Re: Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-20 14:32 ` Re: Injection points: some tools to wait and wake Andrey M. Borodin <x4mmm@yandex-team.ru>
@ 2024-02-20 23:22 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 46+ messages in thread
From: Michael Paquier @ 2024-02-20 23:22 UTC (permalink / raw)
To: Andrey M. Borodin <x4mmm@yandex-team.ru>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>; Ashutosh Bapat <ashutosh.bapat@gmail.com>
On Tue, Feb 20, 2024 at 05:32:38PM +0300, Andrey M. Borodin wrote:
> I will try to simplify test to 2-step, but it would be much easier
> to implement if injection points could be awaken independently.
I don't mind implementing something that wakes only a given point
name, that's just more state data to track in shmem for the module.
> No, "prepared injection point" is not about wait\wake logic. It's
> about having injection point in critical section.
> Normal injection point will pstrdup(name) and fail. In [0] I need a
> test that waits after multixact generation before WAL-logging
> it. It's only possible in a critical section.
It took me some time to understand what you mean here. You are
referring to the allocations done in load_external_function() ->
expand_dynamic_library_name() -> substitute_libpath_macro(), which
is something that has to happen the first time a callback is loaded
into the local cache of a process. So what you want to achieve is to
preload the callback in its cache in a first step without running it,
then be able to run it, so your Prepare[d] layer is just a way to
split InjectionPointRun() into two. Fancy use case. You could
disable the critical section around the INJECTION_POINT() as one
solution, though having something to pre-warm the local cache for a
point name and avoid the allocations done in the external load would
be a second way to achieve that.
"Prepare" is not the best term I would have used, perhaps just have
one INJECTION_POINT_PRELOAD() macro to warm up the cache outside the
critical section with a wrapper routine? You could then leave
InjectionPointRun() as it is now. Having a check on CritSectionCount
in the injection point internals to disable temporarily a critical
section would not feel right as this is used as a safety check
anywhere else.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 46+ messages in thread
end of thread, other threads:[~2024-03-06 22:54 UTC | newest]
Thread overview: 46+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-19 06:01 Injection points: some tools to wait and wake Michael Paquier <michael@paquier.xyz>
2024-02-19 07:51 ` Michael Paquier <michael@paquier.xyz>
2024-02-19 14:28 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 23:28 ` Michael Paquier <michael@paquier.xyz>
2024-02-20 15:55 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-20 22:08 ` Michael Paquier <michael@paquier.xyz>
2024-02-21 07:46 ` Michael Paquier <michael@paquier.xyz>
2024-02-21 11:50 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-22 03:02 ` Michael Paquier <michael@paquier.xyz>
2024-02-22 08:00 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-26 03:57 ` Michael Paquier <michael@paquier.xyz>
2024-02-26 08:24 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 00:55 ` Michael Paquier <michael@paquier.xyz>
2024-02-26 09:10 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-26 23:29 ` Michael Paquier <michael@paquier.xyz>
2024-02-27 06:00 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 11:08 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-27 11:49 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-27 13:39 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-28 04:26 ` Michael Paquier <michael@paquier.xyz>
2024-02-28 05:27 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 05:54 ` Michael Paquier <michael@paquier.xyz>
2024-02-28 06:20 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 05:56 ` Michael Paquier <michael@paquier.xyz>
2024-02-29 08:35 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 09:34 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-29 10:20 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-29 13:19 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-01 02:02 ` Michael Paquier <michael@paquier.xyz>
2024-03-01 06:52 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-03-04 01:44 ` Michael Paquier <michael@paquier.xyz>
2024-03-04 04:17 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 05:16 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-03-04 05:27 ` Michael Paquier <michael@paquier.xyz>
2024-03-04 09:51 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-04 22:23 ` Michael Paquier <michael@paquier.xyz>
2024-03-05 08:43 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 06:17 ` Michael Paquier <michael@paquier.xyz>
2024-03-06 09:19 ` Jelte Fennema-Nio <postgres@jeltef.nl>
2024-03-06 22:54 ` Michael Paquier <michael@paquier.xyz>
2024-03-04 07:22 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-21 08:07 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2024-02-19 08:54 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-19 23:21 ` Michael Paquier <michael@paquier.xyz>
2024-02-20 14:32 ` Andrey M. Borodin <x4mmm@yandex-team.ru>
2024-02-20 23:22 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox