Re: BUG #16663: DROP INDEX did not free up disk space: idle connection hold file marked as deleted

Yes, It's my fault. You're right.

Pavel Borisov <pashkin.elfe@gmail.com> 于2020年11月19日周四 下午11:55写道：

> One thing that doesn't matter is that the modify here seems unnecessary,
>> right?
>>
>> > mdunlinkfork(RelFileNodeBackend rnode, ForkNumber forkNum, bool isRedo)
>> > {
>> > char     *path;
>> > -     int                     ret;
>> > +     int                     ret = 0;
>> > path = relpath(rnode, forkNum
>
>
> I suppose it is indeed necessary as otherwise the result of the comparison
> is not defined in case of 'else' block in the mdunlinkfork() :
> 346     else
> 347     {
> 348         /* Prevent other backends' fds from holding on to the disk
> space */
> 349         do_truncate(path);
> .....
> 356      * Delete any additional segments.
> 357      */
> 358     if (ret >= 0)
> ----------^^^^^^^
>
> --
> Best regards,
> Pavel Borisov
>
> Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
>

So in the present logic, *ret* is always 0 if it is not in recovery mode
(and other *if* conditions are not satisfied). But when the *if* condition
is satisfied, it is possible to skip the deletion of additional segments.
Considering that our goal is to always try to unlink additional segments,
*ret* seems unnecessary here. The code flow looks like:

> if (isRedo || .....)
> {
>     int ret;  /* move to here */
>     ....
> }
> else
> { }
>
> /* Delete any additional segments. */
> if (true)
> ...

Or is there any reason to allow us to skip the attempt to delete additional
segments in recovery mode?