Commit 82c83b33 authored by Alvaro Herrera's avatar Alvaro Herrera

Fix logical_decoding_timelines test crashes

In the test_slot_timelines test module, we were abusing passing NULL
values which was received as zeroes in x86, but this breaks in ARM
(buildfarm member hamster) by crashing instead.  Fix the breakage by
marking these functions as STRICT; the InvalidXid value that was
previously implicit in NULL values (on x86 at least) can now be passed
as 0.  Failing to follow the fmgr protocol to check for NULLs beforehand
was causing ARM to fail, as evidenced by segmentation faults in
buildfarm member hamster.

In order to use the new functionality in the test script, use COALESCE
in the right spot to avoid forwarding NULL values.

This was diagnosed from the hamster crash by Craig Ringer, who also
proposed a different patch (checking for NULL values explicitely in the
C function code, and keeping the non-strictness in the C functions).
I decided to go with this approach instead.
parent 27f3bbfa
......@@ -5,7 +5,7 @@ SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding');
(1 row)
SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location());
SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location());
test_slot_timelines_advance_logical_slot
------------------------------------------
......
......@@ -2,6 +2,6 @@ CREATE EXTENSION test_slot_timelines;
SELECT test_slot_timelines_create_logical_slot('test_slot', 'test_decoding');
SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current(), txid_current(), pg_current_xlog_location(), pg_current_xlog_location());
SELECT test_slot_timelines_advance_logical_slot('test_slot', txid_current()::text::xid, txid_current()::text::xid, pg_current_xlog_location(), pg_current_xlog_location());
SELECT pg_drop_replication_slot('test_slot');
......@@ -3,14 +3,14 @@
CREATE OR REPLACE FUNCTION test_slot_timelines_create_logical_slot(slot_name text, plugin text)
RETURNS void
LANGUAGE c AS 'MODULE_PATHNAME';
STRICT LANGUAGE c AS 'MODULE_PATHNAME';
COMMENT ON FUNCTION test_slot_timelines_create_logical_slot(text, text)
IS 'Create a logical slot at a particular lsn and xid. Do not use in production servers, it is not safe. The slot is created with an invalid xmin and lsn.';
CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin bigint, new_catalog_xmin bigint, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn)
CREATE OR REPLACE FUNCTION test_slot_timelines_advance_logical_slot(slot_name text, new_xmin xid, new_catalog_xmin xid, new_restart_lsn pg_lsn, new_confirmed_lsn pg_lsn)
RETURNS void
LANGUAGE c AS 'MODULE_PATHNAME';
STRICT LANGUAGE c AS 'MODULE_PATHNAME';
COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, bigint, bigint, pg_lsn, pg_lsn)
COMMENT ON FUNCTION test_slot_timelines_advance_logical_slot(text, xid, xid, pg_lsn, pg_lsn)
IS 'Advance a logical slot directly. Do not use this in production servers, it is not safe.';
......@@ -85,8 +85,8 @@ Datum
test_slot_timelines_advance_logical_slot(PG_FUNCTION_ARGS)
{
char *slotname = text_to_cstring(PG_GETARG_TEXT_P(0));
TransactionId new_xmin = (TransactionId) PG_GETARG_INT64(1);
TransactionId new_catalog_xmin = (TransactionId) PG_GETARG_INT64(2);
TransactionId new_xmin = DatumGetTransactionId(PG_GETARG_DATUM(1));
TransactionId new_catalog_xmin = DatumGetTransactionId(PG_GETARG_DATUM(2));
XLogRecPtr restart_lsn = PG_GETARG_LSN(3);
XLogRecPtr confirmed_lsn = PG_GETARG_LSN(4);
......@@ -95,7 +95,7 @@ test_slot_timelines_advance_logical_slot(PG_FUNCTION_ARGS)
ReplicationSlotAcquire(slotname);
if (MyReplicationSlot->data.database != MyDatabaseId)
elog(ERROR, "Trying to update a slot on a different database");
elog(ERROR, "trying to update a slot on a different database");
MyReplicationSlot->data.xmin = new_xmin;
MyReplicationSlot->data.catalog_xmin = new_catalog_xmin;
......
......@@ -172,8 +172,13 @@ is($stdout, '', 'No slots exist on the replica');
# we're just doing it by hand for this test. This is exposing
# postgres innards to SQL so it's unsafe except for testing.
$node_master->safe_psql('postgres', 'CREATE EXTENSION test_slot_timelines;');
my $slotinfo = $node_master->safe_psql('postgres',
'SELECT slot_name, plugin, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn FROM pg_replication_slots ORDER BY slot_name'
my $slotinfo = $node_master->safe_psql(
'postgres',
qq{SELECT slot_name, plugin,
COALESCE(xmin, '0'), catalog_xmin,
restart_lsn, confirmed_flush_lsn
FROM pg_replication_slots ORDER BY slot_name}
);
diag "Copying slots to replica";
open my $fh, '<', \$slotinfo or die $!;
......@@ -183,10 +188,7 @@ while (<$fh>)
chomp $_;
my ($slot_name, $plugin, $xmin, $catalog_xmin, $restart_lsn,
$confirmed_flush_lsn)
= map {
if ($_ ne '') { "'$_'" }
else { 'NULL' }
} split qr/\|/, $_;
= map { "'$_'" } split qr/\|/, $_;
print
"# Copying slot $slot_name,$plugin,$xmin,$catalog_xmin,$restart_lsn,$confirmed_flush_lsn\n";
......@@ -208,7 +210,7 @@ is( $stdout,
$stdout = $node_replica->safe_psql(
'postgres',
qq{SELECT slot_name, plugin, xmin, catalog_xmin,
qq{SELECT slot_name, plugin, COALESCE(xmin, '0'), catalog_xmin,
restart_lsn, confirmed_flush_lsn
FROM pg_replication_slots
ORDER BY slot_name});
......@@ -243,6 +245,7 @@ diag "oldest needed xlog seg is $oldest_needed_segment ";
opendir(my $pg_xlog, $node_master->data_dir . "/pg_xlog") or die $!;
while (my $seg = readdir $pg_xlog)
{
next if $seg eq '.' or $seg eq '..';
next unless $seg >= $oldest_needed_segment && $seg =~ /^[0-9]{24}/;
diag "copying xlog seg $seg";
copy(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment