It seems to me reasonable to move size check above CRC computation. However, it
seems suspicious to me to run a test that allocates 1Gb in `make check`. Maybe,
there are places that are not exercised too often. Perhaps recovery tests or
something like that.
Hi Andrey,
I share your concern about memory consumption and also agree recovery
tests look like the right place for this test.
So I split the patch into two - the change proper and the converted TAP
test, guarded by PG_TEST_EXTRA. Attaching both patches.
---
Sergey
From c3818555611ab4c9c39dee90e9e56e69c2fcc1ed Mon Sep 17 00:00:00 2001
From: Sergey Fukanchik <s.fukanc...@postgrespro.ru>
Date: Sun, 7 Sep 2025 12:06:03 +0300
Subject: [PATCH v2 1/2] Perform check for oversized WAL record before
calculating record CRC
---
src/backend/access/transam/xloginsert.c | 26 ++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index c7571429e8e..e8c55924c12 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -904,19 +904,6 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
hdr_rdt.len = (scratch - hdr_scratch);
total_len += hdr_rdt.len;
- /*
- * Calculate CRC of the data
- *
- * Note that the record header isn't added into the CRC initially since we
- * don't know the prev-link yet. Thus, the CRC will represent the CRC of
- * the whole record in the order: rdata, then backup blocks, then record
- * header.
- */
- INIT_CRC32C(rdata_crc);
- COMP_CRC32C(rdata_crc, hdr_scratch + SizeOfXLogRecord, hdr_rdt.len - SizeOfXLogRecord);
- for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
- COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
-
/*
* Ensure that the XLogRecord is not too large.
*
@@ -930,6 +917,19 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
errdetail_internal("WAL record would be %" PRIu64 " bytes (of maximum %u bytes); rmid %u flags %u.",
total_len, XLogRecordMaxSize, rmid, info)));
+ /*
+ * Calculate CRC of the data
+ *
+ * Note that the record header isn't added into the CRC initially since we
+ * don't know the prev-link yet. Thus, the CRC will represent the CRC of
+ * the whole record in the order: rdata, then backup blocks, then record
+ * header.
+ */
+ INIT_CRC32C(rdata_crc);
+ COMP_CRC32C(rdata_crc, hdr_scratch + SizeOfXLogRecord, hdr_rdt.len - SizeOfXLogRecord);
+ for (rdt = hdr_rdt.next; rdt != NULL; rdt = rdt->next)
+ COMP_CRC32C(rdata_crc, rdt->data, rdt->len);
+
/*
* Fill in the fields in the record header. Prev-link is filled in later,
* once we know where in the WAL the record will be inserted. The CRC does
--
2.34.1
From 23172b798c82b039ec5be178a3740362ae65fc37 Mon Sep 17 00:00:00 2001
From: Sergey Fukanchik <s.fukanc...@postgrespro.ru>
Date: Sun, 7 Sep 2025 12:06:27 +0300
Subject: [PATCH v2 2/2] Add a TAP test for oversized WAL records
---
.../recovery/t/049_oversized_wal_record.pl | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
create mode 100644 src/test/recovery/t/049_oversized_wal_record.pl
diff --git a/src/test/recovery/t/049_oversized_wal_record.pl b/src/test/recovery/t/049_oversized_wal_record.pl
new file mode 100644
index 00000000000..8604ef53d48
--- /dev/null
+++ b/src/test/recovery/t/049_oversized_wal_record.pl
@@ -0,0 +1,50 @@
+
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+# Try to create a WAL record which is larger than the limit XLogRecordMaxSize.
+# That should raise an error.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+# Disable the test unless requested explicitly because it requires 1Gb of memory
+if (exists $ENV{PG_TEST_EXTRA}
+ && $ENV{PG_TEST_EXTRA} =~ m/\boversized_wal_record\b/)
+{
+ plan tests => 4;
+}
+else
+{
+ plan skip_all =>
+ 'Skipping oversized_wal_record test as it requires a lot of memory';
+}
+
+note "Check handing of oversized WAL records";
+
+# Initialize and start basic node
+my $node = PostgreSQL::Test::Cluster->new('node');
+
+$node->init;
+
+$node->start;
+
+# Insert oversized record
+my ($ret, $stdout, $stderr) = $node->psql(
+ 'postgres',
+ "select pg_logical_emit_message(false, 'a', repeat('00000000', (1020 * 1024 * 1024)/8+1), true)",
+ on_error_die => 0
+ );
+
+# Verify we have error
+ok($ret != 0, 'exit code is non-zero');
+
+ok($stderr =~ qr/ERROR: oversized WAL record/, 'expect error');
+
+ok($stderr =~ qr/DETAIL: WAL record would be [0-9]+ bytes \(of maximum [0-9]+ bytes\);/,
+ 'error details');
+
+$node->stop;
+
+done_testing();
--
2.34.1
