Re: Proposal: remove obsolete hot-standby testing infrastructure

Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> writes:
> On 03.01.22 22:50, Tom Lane wrote:
>> The attached proposed patch removes some ancient infrastructure for
>> manually testing hot standby.

> I looked into this some time ago and concluded that this test contains a
> significant amount of testing that isn't obviously done anywhere else.
> I don't have the notes anymore, and surely some things have progressed
> since, but I wouldn't just throw the old test suite away without
> actually checking.

Fair enough ... so I looked, and there's not much at all that
I'm worried about.

hs_standby_allowed:
This is basically checking that the standby can see data from
the primary, which we surely have covered. Although it does
also cover propagation of nextval, which AFAICS is not tested
in src/test/recovery, so perhaps that's worth troubling over.

There are also some checks that particular commands are allowed
on the standby, which seem to me to be not too helpful;
see also comments on the next file.

hs_standby_disallowed:
Inverse of the above: check that some commands are disallowed.
We check some of these in 001_stream_rep.pl, and given the current
code structure in utility.c (ClassifyUtilityCommandAsReadOnly etc),
I do not see much point in adding more test cases of the same sort.
The only likely new bug in that area would be misclassification of
some new command, and no amount of testing of existing cases will
catch that.

There are also tests that particular functions are disallowed, which
isn't something that goes through ClassifyUtilityCommandAsReadOnly.
Nonetheless, adding more test cases here wouldn't help catch future
oversights of that type, so I remain unexcited.

hs_standby_functions:
Mostly also checking that things are disallowed. There's also
a test of pg_cancel_backend, which is cute but probably suffers
from timing instability (i.e., delayed arrival of the signal
might change the output). Moreover, pg_cancel_backend is already
covered in the isolation tests, and I see no reason to think
it'd operate differently on a standby.

hs_standby_check:
Checks pg_is_in_recovery(), which is checked far more thoroughly
by pg_ctl/t/003_promote.pl.

hs_primary_extremes:
Checks that we can cope with deep subtransaction nesting.
Maybe this is worth preserving, but I sort of doubt it ---
the standby doesn't even see the nesting does it?
Also checks that the standby can cope with 257 exclusive
locks at once, which corresponds to no interesting limit
that I know of.

So basically, I'd be inclined to add a couple of tests of
sequence-update propagation to src/test/recovery and
call it good.

regards, tom lane