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); }