On Tue, Feb 13, 2024 at 2:11 AM Bharath Rupireddy
<bharath.rupireddyforpostg...@gmail.com> wrote:
>
> Hi,
>
> Postgres has a good amount of code for dealing with backtraces - two
> GUCs backtrace_functions and backtrace_on_internal_error,
> errbacktrace; all of which use core function set_backtrace from
> elog.c. I've not seen this code being tested at all, see code coverage
> report -
> https://coverage.postgresql.org/src/backend/utils/error/elog.c.gcov.html.
>
> I think adding a simple test module (containing no .c files) with only
> TAP tests will help cover this code. I ended up having it as a
> separate module under src/test/modules/test_backtrace as I was not
> able to find an existing TAP file in src/test to add these tests. I'm
> able to verify the backtrace related code with the attached patch
> consistently. The TAP tests rely on the fact that the server emits
> text "BACKTRACE: " to server logs before logging the backtrace, and
> the backtrace contains the function name in which the error occurs.
> I've turned off query statement logging (set log_statement = none,
> log_min_error_statement = fatal) so that the tests get to see the
> functions only in the backtrace. Although the CF bot is happy with the
> attached patch
> https://github.com/BRupireddy2/postgres/tree/add_test_module_for_bcktrace_functionality_v1,
> there might be some more flakiness to it.
>
> Thoughts?
Ran pgperltidy on the new TAP test file added. Please see the attached v2 patch.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 2cfe9c87dea980efa2f319e9c2e873e83e561b9e Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Tue, 20 Feb 2024 05:48:41 +0000
Subject: [PATCH v2] Add test module for backtrace functionality
---
src/test/modules/Makefile | 1 +
src/test/modules/meson.build | 1 +
src/test/modules/test_backtrace/.gitignore | 4 +
src/test/modules/test_backtrace/Makefile | 14 +++
src/test/modules/test_backtrace/README | 4 +
src/test/modules/test_backtrace/meson.build | 12 +++
.../modules/test_backtrace/t/001_backtrace.pl | 98 +++++++++++++++++++
7 files changed, 134 insertions(+)
create mode 100644 src/test/modules/test_backtrace/.gitignore
create mode 100644 src/test/modules/test_backtrace/Makefile
create mode 100644 src/test/modules/test_backtrace/README
create mode 100644 src/test/modules/test_backtrace/meson.build
create mode 100644 src/test/modules/test_backtrace/t/001_backtrace.pl
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 89aa41b5e3..3967311048 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -13,6 +13,7 @@ SUBDIRS = \
libpq_pipeline \
plsample \
spgist_name_ops \
+ test_backtrace \
test_bloomfilter \
test_copy_callbacks \
test_custom_rmgrs \
diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build
index 8fbe742d38..3b21b389af 100644
--- a/src/test/modules/meson.build
+++ b/src/test/modules/meson.build
@@ -12,6 +12,7 @@ subdir('libpq_pipeline')
subdir('plsample')
subdir('spgist_name_ops')
subdir('ssl_passphrase_callback')
+subdir('test_backtrace')
subdir('test_bloomfilter')
subdir('test_copy_callbacks')
subdir('test_custom_rmgrs')
diff --git a/src/test/modules/test_backtrace/.gitignore b/src/test/modules/test_backtrace/.gitignore
new file mode 100644
index 0000000000..5dcb3ff972
--- /dev/null
+++ b/src/test/modules/test_backtrace/.gitignore
@@ -0,0 +1,4 @@
+# Generated subdirectories
+/log/
+/results/
+/tmp_check/
diff --git a/src/test/modules/test_backtrace/Makefile b/src/test/modules/test_backtrace/Makefile
new file mode 100644
index 0000000000..b908864e89
--- /dev/null
+++ b/src/test/modules/test_backtrace/Makefile
@@ -0,0 +1,14 @@
+# src/test/modules/test_backtrace/Makefile
+
+TAP_TESTS = 1
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/test_backtrace
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
diff --git a/src/test/modules/test_backtrace/README b/src/test/modules/test_backtrace/README
new file mode 100644
index 0000000000..ae1366bd58
--- /dev/null
+++ b/src/test/modules/test_backtrace/README
@@ -0,0 +1,4 @@
+This directory doesn't actually contain any extension module.
+
+Instead it is a home for backtrace tests that we want to run using TAP
+infrastructure.
diff --git a/src/test/modules/test_backtrace/meson.build b/src/test/modules/test_backtrace/meson.build
new file mode 100644
index 0000000000..4adffcc914
--- /dev/null
+++ b/src/test/modules/test_backtrace/meson.build
@@ -0,0 +1,12 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+tests += {
+ 'name': 'test_backtrace',
+ 'sd': meson.current_source_dir(),
+ 'bd': meson.current_build_dir(),
+ 'tap': {
+ 'tests': [
+ 't/001_backtrace.pl',
+ ],
+ },
+}
diff --git a/src/test/modules/test_backtrace/t/001_backtrace.pl b/src/test/modules/test_backtrace/t/001_backtrace.pl
new file mode 100644
index 0000000000..414af436ae
--- /dev/null
+++ b/src/test/modules/test_backtrace/t/001_backtrace.pl
@@ -0,0 +1,98 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# Test PostgreSQL backtrace related code.
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+
+# Turn off query statement logging so that we get to see the functions in only
+# backtraces.
+$node->append_conf(
+ 'postgresql.conf', qq{
+backtrace_functions = 'pg_terminate_backend, pg_create_restore_point'
+wal_level = replica
+log_statement = none
+log_min_error_statement = fatal
+});
+$node->start;
+
+# Check if backtrace generation is supported on this installation.
+my ($result, $stdout, $stderr);
+my $log_offset = -s $node->logfile;
+
+# Generate an error with negative timeout.
+($result, $stdout, $stderr) = $node->psql(
+ 'postgres', q(
+ SELECT pg_terminate_backend(123, -1);
+));
+
+if ($stderr =~ m/"timeout" must not be negative/
+ && $node->log_contains(
+ qr/backtrace generation is not supported by this installation/,
+ $log_offset))
+{
+ plan skip_all =>
+ 'backtrace generation is not supported by this installation';
+}
+
+# Start verifying for the results only after skip_all check.
+ok( $stderr =~ m/"timeout" must not be negative/,
+ 'error from terminating backend is logged');
+
+my $backtrace_text = qr/BACKTRACE: /;
+
+ok( $node->log_contains($backtrace_text, $log_offset),
+ "backtrace pg_terminate_backend start is found");
+
+ok($node->log_contains("pg_terminate_backend", $log_offset),
+ "backtrace pg_terminate_backend is found");
+
+$log_offset = -s $node->logfile;
+
+# Generate an error with a long restore point name.
+($result, $stdout, $stderr) = $node->psql(
+ 'postgres', q(
+ SELECT pg_create_restore_point(repeat('A', 1024));
+));
+ok( $stderr =~ m/value too long for restore point \(maximum .* characters\)/,
+ 'error from restore point function is logged');
+
+ok( $node->log_contains($backtrace_text, $log_offset),
+ "backtrace pg_create_restore_point start is found");
+
+ok($node->log_contains("pg_create_restore_point", $log_offset),
+ "backtrace pg_create_restore_point is found");
+
+# Test if backtrace gets generated on internal errors when
+# backtrace_on_internal_error is set. Note that we use a function (i.e.
+# pg_replication_slot_advance) that generates an error with ereport without
+# setting errcode explicitly, in which case elog.c assumes it as an internal
+# error (see edata->sqlerrcode = ERRCODE_INTERNAL_ERROR; in errstart() in
+# elog.c).
+$node->append_conf('postgresql.conf', "backtrace_on_internal_error = on");
+$node->reload;
+
+$node->safe_psql('postgres',
+ "SELECT pg_create_physical_replication_slot('myslot', true);");
+
+$log_offset = -s $node->logfile;
+
+($result, $stdout, $stderr) = $node->psql(
+ 'postgres', q(
+ SELECT pg_replication_slot_advance('myslot', '0/0'::pg_lsn);
+));
+ok($stderr =~ m/invalid target WAL LSN/,
+ 'error from replication slot advance is logged');
+
+ok( $node->log_contains($backtrace_text, $log_offset),
+ "backtrace pg_replication_slot_advance start is found");
+
+ok($node->log_contains("pg_replication_slot_advance", $log_offset),
+ "backtrace pg_replication_slot_advance is found");
+
+done_testing();
--
2.34.1
