Re: Infinite loop in XLogPageRead() on standby - Mailing list pgsql-hackers

From Alexander Kukushkin
Subject Re: Infinite loop in XLogPageRead() on standby
Date
Msg-id CAFh8B==5hgsabTjMNgvjDJcf2XY8ZOm6mTWuhSwnzYVgXL1oJw@mail.gmail.com
Whole thread Raw
In response to Re: Infinite loop in XLogPageRead() on standby  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
List pgsql-hackers
Hi Michael,

On Wed, 15 Jan 2025 at 05:45, Michael Paquier <michael@paquier.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

pgsql-hackers by date:

Previous
From: Andy Fan
Date:
Subject: Purpose of wal_init_zero
Next
From: Ajin Cherian
Date:
Subject: Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility.