Re: Infinite loop in XLogPageRead() on standby

From: Alexander Kukushkin <cyberdemn(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Ants Aasma <ants(dot)aasma(at)cybertec(dot)at>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, thomas(dot)munro(at)gmail(dot)com
Subject: Re: Infinite loop in XLogPageRead() on standby
Date: 2025-01-15 09:35:42
Message-ID: CAFh8B==5hgsabTjMNgvjDJcf2XY8ZOm6mTWuhSwnzYVgXL1oJw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael,

On Wed, 15 Jan 2025 at 05:45, Michael Paquier <michael(at)paquier(dot)xyz> wrote:.

> The new regression test is something I really want to keep around,
> to be able to emulate the infinite loop, but I got annoyed with the
> amount of duplication between the new test and the existing
> 039_end_of_wal.pl as there share the same ideas. I have refactored
> 039_end_of_wal.pl and moved its routines to generate WAL messages,
> to advance WAL and to write WAL into Cluster.pm, then reused the
> refactored result in the new test.
>

> Barring objections, I'd like to get the main issue in 0002 fixed at
> the beginning of next week. I am also planning to do the refactoring
> bits tomorrow or so as these are rather straight-forward. The names
> of the new routines in Cluster.pm are inherited from the existing
> recovery test. Perhaps they could use a better name, but 0001 does
> not look that bad to me either.
>

Thank you for picking it up. I briefly looked at both patches. The actual
fix in XLogPageRead() looks good to me.
I also agree with suggested refactoring, where there is certainly some room
for improvement - $WAL_SEGMENT_SIZE, $WAL_BLOCK_SIZE variables and
get_int_setting(), start_of_page() funcs are still duplicated in both test
files.
Maybe we can have something like the following in Cluster.pm? It'll allow
further simplify tests and reduce code duplication.

sub get_timeline_id
{
my self = shift;
return $self->safe_psql('postgres',
"SELECT timeline_id FROM pg_control_checkpoint()");
}

sub get_int_setting
{
my ($self, $name) = @_;
return int(
$self->safe_psql(
'postgres',
"SELECT setting FROM pg_settings WHERE name =
'$name'"));
}

sub WAL_SEGMENT_SIZE
{
my $self = shift;
self->{_WAL_SEGMENT_SIZE} = self->get_int_setting('wal_segment_size')
unless defined self->{_WAL_SEGMENT_SIZE};
return self->{_WAL_SEGMENT_SIZE};
}

sub WAL_BLOCK_SIZE
{
my $self = shift;
self->{_WAL_BLOCK_SIZE} = self->get_int_setting('wal_block_size')
unless defined self->{_WAL_BLOCK_SIZE};
return self->{_WAL_BLOCK_SIZE};
}

sub start_of_page
{
my ($self, $lsn) = @_;
return $lsn & ~($self->WAL_BLOCK_SIZE - 1);
}

Regards,
--
Alexander Kukushkin

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ilia Evdokimov 2025-01-15 09:47:33 Re: Sample rate added to pg_stat_statements
Previous Message Michael Paquier 2025-01-15 09:27:22 Re: per backend I/O statistics