On Thu, Jul 7, 2022 at 10:58 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> But if many think that it's worth adding the test, I will give a try. But even in that case, I think it's better to
committhe proposed patch at first to fix the bug, and then to write the patch adding the test.
I don't think that we necessarily need to have a test for this patch.
It's true that we don't really have good test coverage of write-ahead
logging and recovery, but this doesn't seem like the most important
thing to be testing in that area, either, and developing stable tests
for stuff like this can be a lot of work.
I do kind of feel like the patch is fixing two separate bugs. The
change to SendBaseBackup() is fixing the problem that, because there's
SQL access on replication connections, we could try to start a backup
in the middle of another backup by mixing and matching the two
different methods of doing backups. The change to do_pg_abort_backup()
is fixing the fact that, after aborting a base backup, we don't reset
the session state properly so that another backup can be tried
afterwards.
I don't know if it's worth committing them separately - they are very
small fixes. But it would probably at least be good to highlight in
the commit message that there are two different issues.
--
Robert Haas
EDB: http://www.enterprisedb.com