Re: Speedup of relation deletes during recovery

On Fri, Mar 30, 2018 at 11:46 AM, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Fri, Mar 30, 2018 at 11:19:58AM +0900, Kyotaro HORIGUCHI wrote:
>> At Fri, 30 Mar 2018 08:31:29 +0900, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote in <CAHGQGwHVQkdfDqtvGVkty+19cQakAydXn1etGND3X0PHbZ3+6w(at)mail(dot)gmail(dot)com>
>>> When multiple relations are deleted at the same transaction,
>>> the files of those relations are deleted by one call to smgrdounlinkall(),
>>> which leads to scan whole shared_buffers only one time. OTOH,
>>> during recovery, smgrdounlink() (not smgrdounlinkall()) is called
>>> for each file to delete, which leads to scan shared_buffers multiple times.
>>> Obviously this can cause to increase the WAL replay time very much
>>> especially when shared_buffers is huge.
>>>
>>> To alleviate this situation, I'd like to propose to change the recovery
>>> so that it also calls smgrdounlinkall() only one time to delete multiple
>>> relation files. Patch attached. Thought?
>>
>> It is obviously a left-over of 279628a0a7. This patch applies the
>> same change with the patch and looks fine for me. Note that
>> FinishPreparedTransaction has the same loop over smgrdounlink.

Thanks for the review! I also changed FinishPreparedTransaction() so that
it uses smgrdounlinkall(). Patch attached.

> Yeah, I was just going to say the same after looking at Fujii-san's
> patch. This would also cause smgrdounlink() to become unused in the
> core code. So this could just be... Removed?

Yes, I think. And, I found that smgrdounlinkfork() is also dead code.
Per the discussion [1], this unused function was left intentionally.
But it's still dead code since 2012, so I'd like to remove it. Patch attached.

[1]
https://www.postgresql.org/message-id/1471.1339106082@sss.pgh.pa.us

Regards,

--
Fujii Masao