Re: Backup command and functions can cause assertion failure and segmentation fault

On 2022/07/01 12:05, Kyotaro Horiguchi wrote:
> At Fri, 01 Jul 2022 11:56:14 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
>> At Fri, 01 Jul 2022 11:46:53 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in
>>> Please find the attached.
>>
>> Mmm. It forgot the duplicate-call prevention and query-cancel
>> handling... The first one is the same as you posted but the second one
>> is still a problem..
> 
> So this is the first cut of that.

Thanks for reviewing the patch!

+    PG_FINALLY();
+    {
          endptr = do_pg_backup_stop(labelfile->data, !opt->nowait, &endtli);
      }
-    PG_END_ENSURE_ERROR_CLEANUP(do_pg_abort_backup, BoolGetDatum(false));
-
+    PG_END_TRY();

This change makes perform_base_backup() call do_pg_backup_stop() even when an error is reported while taking a backup,
i.e.,between PG_TRY() and PG_FINALLY(). Why do_pg_backup_stop() needs to be called in such an error case? It not only
cleansup the backup state but also writes the backup-end WAL record, waits for WAL archiving. In an error case, I think
thatonly the cleanup of the backup state is necessary. So it seems ok to use do_pg_abort_backup() in that case, as it
isfor now.
 

So I'm still thinking that the patch I posted is simpler and enough.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION