Peter Eisentraut <peter.eisentr...@enterprisedb.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