Horton

Thread

Commits

  1. Use truncate(2) where appropriate.

  2. Free disk space for dropped relations on commit.

  3. Rename the "point is strictly above/below point" comparison operators.

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

    The Post Office <noreply@postgresql.org> — 2020-10-09T13:24:15Z

    The following bug has been logged on the website:

Bug reference:      16663
Logged by:          Denis Patron
Email address:      denis.patron@previnet.it
PostgreSQL version: 11.9
Operating system:   CentOS 7
Description:        

I have an index, which at the file system level, is made up of multiple
segments (file: <id>.1, <id>.2 ecc). When I DROP INDEX, the index is dropped
in Postgresql but at the file system level, the segments are marked as
"deleted". if I check with the lsof command, I see that the segments are in
use from an idle connection. This does not happen if the index is formed by
only one segment (in my case <1Gb). How can I prevent this?
thanks

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-10-14T03:05:10Z

    This is not a bug.

At Fri, 09 Oct 2020 13:24:15 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in 
> The following bug has been logged on the website:
> 
> Bug reference:      16663
> Logged by:          Denis Patron
> Email address:      denis.patron@previnet.it
> PostgreSQL version: 11.9
> Operating system:   CentOS 7
> Description:        
> 
> I have an index, which at the file system level, is made up of multiple
> segments (file: <id>.1, <id>.2 ecc). When I DROP INDEX, the index is dropped
> in Postgresql but at the file system level, the segments are marked as
> "deleted". if I check with the lsof command, I see that the segments are in
> use from an idle connection. This does not happen if the index is formed by
> only one segment (in my case <1Gb). How can I prevent this?
> thanks

That references to deleted files will dissapear at the beginning of
the next transaction.

At the time a relation including an index is dropped, the first
segment file (named as "<id>" without a suffix number) is left behind
so the file is not shown as "(deleted)" in lsof output.

The next checkpoint removes the first segment.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    Andres Freund <andres@anarazel.de> — 2020-10-14T04:35:40Z

    Hi,

On 2020-10-14 12:05:10 +0900, Kyotaro Horiguchi wrote:
> This is not a bug.
>
> At Fri, 09 Oct 2020 13:24:15 +0000, PG Bug reporting form <noreply@postgresql.org> wrote in
> > The following bug has been logged on the website:
> >
> > Bug reference:      16663
> > Logged by:          Denis Patron
> > Email address:      denis.patron@previnet.it
> > PostgreSQL version: 11.9
> > Operating system:   CentOS 7
> > Description:
> >
> > I have an index, which at the file system level, is made up of multiple
> > segments (file: <id>.1, <id>.2 ecc). When I DROP INDEX, the index is dropped
> > in Postgresql but at the file system level, the segments are marked as
> > "deleted". if I check with the lsof command, I see that the segments are in
> > use from an idle connection. This does not happen if the index is formed by
> > only one segment (in my case <1Gb). How can I prevent this?
> > thanks
>
> That references to deleted files will dissapear at the beginning of
> the next transaction.
>
> At the time a relation including an index is dropped, the first
> segment file (named as "<id>" without a suffix number) is left behind
> so the file is not shown as "(deleted)" in lsof output.

I think we should consider either occasionally sending a sinval catchup
interrupt to backends that have been idle for a while, or to use a timer
that we use to limit the maximum time until we process sinvals. Just
having to wait till all backends become busy and process sinval events
doesn't really seem like good approach to me.

Regards,

Andres

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

    denis.patron <denis.patron@previnet.it> — 2020-10-14T06:47:34Z

    Andres Freund wrote
> Hi,
> 
> On 2020-10-14 12:05:10 +0900, Kyotaro Horiguchi wrote:
>> This is not a bug.
>>
>> At Fri, 09 Oct 2020 13:24:15 +0000, PG Bug reporting form &lt;

> noreply@

> &gt; wrote in
>> > The following bug has been logged on the website:
>> >
>> > Bug reference:      16663
>> > Logged by:          Denis Patron
>> > Email address:      

> denis.patron@

>> > PostgreSQL version: 11.9
>> > Operating system:   CentOS 7
>> > Description:
>> >
>> > I have an index, which at the file system level, is made up of multiple
>> > segments (file: 
> <id>
> .1, 
> <id>
> .2 ecc). When I DROP INDEX, the index is dropped
>> > in Postgresql but at the file system level, the segments are marked as
>> > "deleted". if I check with the lsof command, I see that the segments
>> are in
>> > use from an idle connection. This does not happen if the index is
>> formed by
>> > only one segment (in my case <1Gb). How can I prevent this?
>> > thanks
>>
>> That references to deleted files will dissapear at the beginning of
>> the next transaction.
>>
>> At the time a relation including an index is dropped, the first
>> segment file (named as "
> <id>
> " without a suffix number) is left behind
>> so the file is not shown as "(deleted)" in lsof output.
> 
> I think we should consider either occasionally sending a sinval catchup
> interrupt to backends that have been idle for a while, or to use a timer
> that we use to limit the maximum time until we process sinvals. Just
> having to wait till all backends become busy and process sinval events
> doesn't really seem like good approach to me.
> 
> Regards,
> 
> Andres



thanks for replying.
the problem is that I have a very large database, with indexes of up to 70
Gb. while I redo the indexes in concurrently mode, if an idle transaction is
using the index in question, the segment file (<id> _1 <id> _2 etc) of the
index remains in the filesystem (marked as deleted) as long as the idle
connection that it is blocking it does not make another transaction. this
means that I can have hundreds of GB of space occupied by files marked
"deleted", and this for hours. the risk is to run out of free space



--
Sent from: https://www.postgresql-archive.org/PostgreSQL-bugs-f2117394.html

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-10-14T19:08:06Z

    On Wed, Oct 14, 2020 at 5:35 PM Andres Freund <andres@anarazel.de> wrote:
> On 2020-10-14 12:05:10 +0900, Kyotaro Horiguchi wrote:
> > At the time a relation including an index is dropped, the first
> > segment file (named as "<id>" without a suffix number) is left behind
> > so the file is not shown as "(deleted)" in lsof output.
>
> I think we should consider either occasionally sending a sinval catchup
> interrupt to backends that have been idle for a while, or to use a timer
> that we use to limit the maximum time until we process sinvals. Just
> having to wait till all backends become busy and process sinval events
> doesn't really seem like good approach to me.

Oops, I also replied to this but now I see that I accidentally replied
only to Horiguchi-san and not the list!  I was thinking that we should
perhaps consider truncating the files to give back the disk space (as
we do for the first segment), so that it doesn't matter so much how
long other backends take to process SHAREDINVALSMGR_ID, close their
descriptors and release the inode.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2020-10-14T19:14:58Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> On Wed, Oct 14, 2020 at 5:35 PM Andres Freund <andres@anarazel.de> wrote:
>> I think we should consider either occasionally sending a sinval catchup
>> interrupt to backends that have been idle for a while, or to use a timer
>> that we use to limit the maximum time until we process sinvals. Just
>> having to wait till all backends become busy and process sinval events
>> doesn't really seem like good approach to me.

> Oops, I also replied to this but now I see that I accidentally replied
> only to Horiguchi-san and not the list!  I was thinking that we should
> perhaps consider truncating the files to give back the disk space (as
> we do for the first segment), so that it doesn't matter so much how
> long other backends take to process SHAREDINVALSMGR_ID, close their
> descriptors and release the inode.

+1, I was also thinking that.  It'd be pretty easy to fit into the
existing system structure (I think, without having looked at the relevant
code lately), and it would not add any overhead to normal processing.
Installing a timeout to handle this per Andres' idea inevitably *would*
add overhead.

			regards, tom lane

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-10-15T01:26:36Z

    On Thu, Oct 15, 2020 at 8:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > On Wed, Oct 14, 2020 at 5:35 PM Andres Freund <andres@anarazel.de> wrote:
> >> I think we should consider either occasionally sending a sinval catchup
> >> interrupt to backends that have been idle for a while, or to use a timer
> >> that we use to limit the maximum time until we process sinvals. Just
> >> having to wait till all backends become busy and process sinval events
> >> doesn't really seem like good approach to me.
>
> > Oops, I also replied to this but now I see that I accidentally replied
> > only to Horiguchi-san and not the list!  I was thinking that we should
> > perhaps consider truncating the files to give back the disk space (as
> > we do for the first segment), so that it doesn't matter so much how
> > long other backends take to process SHAREDINVALSMGR_ID, close their
> > descriptors and release the inode.
>
> +1, I was also thinking that.  It'd be pretty easy to fit into the
> existing system structure (I think, without having looked at the relevant
> code lately), and it would not add any overhead to normal processing.
> Installing a timeout to handle this per Andres' idea inevitably *would*
> add overhead.

Alright, here is a first swing at making our behaviour more consistent
in two ways:

1.  The first segment should be truncated even in recovery.
2.  Later segments should be truncated on commit.

I don't know why the existing coding decides not to try to unlink the
later segments if the truncate of segment 0 failed.  We already
committed, we should plough on.

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

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-10-15T01:42:48Z

    Ouch. You beat me to it.

At Thu, 15 Oct 2020 14:26:36 +1300, Thomas Munro <thomas.munro@gmail.com> wrote in 
> On Thu, Oct 15, 2020 at 8:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Thomas Munro <thomas.munro@gmail.com> writes:
> > > On Wed, Oct 14, 2020 at 5:35 PM Andres Freund <andres@anarazel.de> wrote:
> > >> I think we should consider either occasionally sending a sinval catchup
> > >> interrupt to backends that have been idle for a while, or to use a timer
> > >> that we use to limit the maximum time until we process sinvals. Just
> > >> having to wait till all backends become busy and process sinval events
> > >> doesn't really seem like good approach to me.
> >
> > > Oops, I also replied to this but now I see that I accidentally replied
> > > only to Horiguchi-san and not the list!  I was thinking that we should
> > > perhaps consider truncating the files to give back the disk space (as
> > > we do for the first segment), so that it doesn't matter so much how
> > > long other backends take to process SHAREDINVALSMGR_ID, close their
> > > descriptors and release the inode.
> >
> > +1, I was also thinking that.  It'd be pretty easy to fit into the
> > existing system structure (I think, without having looked at the relevant
> > code lately), and it would not add any overhead to normal processing.
> > Installing a timeout to handle this per Andres' idea inevitably *would*
> > add overhead.
> 
> Alright, here is a first swing at making our behaviour more consistent
> in two ways:
> 
> 1.  The first segment should be truncated even in recovery.
> 2.  Later segments should be truncated on commit.
> 
> I don't know why the existing coding decides not to try to unlink the
> later segments if the truncate of segment 0 failed.  We already
> committed, we should plough on.

I was trying the almost the same thing except how to emit the error
message for truncation and not trying to unlink if truncation ends
with ENOENT for following segments.

regareds.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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

    dpat <denni.pat@gmail.com> — 2020-10-15T06:57:11Z

    thanks for the patch. 
Do you think it can be included in the next minor releases or the only
solution will be to recompile?
regards
Denis



--
Sent from: https://www.postgresql-archive.org/PostgreSQL-bugs-f2117394.html

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-10-15T23:54:32Z

    On Thu, Oct 15, 2020 at 8:20 PM denni.pat <denni.pat@gmail.com> wrote:
> thanks for the patch.
> Do you think it can be included in the next minor releases or the only
> solution will be to recompile?

I would vote +1 for back-patching a fix for this problem (that is,
pushing it into the minor releases), because I agree that it's very
arguably a bug that we treat the segments differently, and looking
around I do see reports of people having to terminate processes to get
their disk space back.  I'd definitely want a consensus on that plan
from some experienced reviewers and testers, though.  For anyone
wanting to test this, you might want to set RELSEGSIZE to a smaller
number in src/include/pg_config.h.

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

    Pavel Borisov <pashkin.elfe@gmail.com> — 2020-11-11T14:13:13Z

    Thomas,
I get into the patch and I think it's worth being committed and
backpatched.
BTW I noticed that sometimes the same comparisons are done twice, and I
made a very minor refactor of the code. PFA v2 of a patch if you don't mind.
As for the question on what to do with the additional segments if the first
one failed to be truncated, I don't consider myself experienced enough and
surely someone else's independent opinion is very much welcome.

-- 
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>

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

    Neil Chen <carpenter.nail.cz@gmail.com> — 2020-11-19T08:20:18Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Hi, I have tested the feature and it worked well. 
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

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

    Pavel Borisov <pashkin.elfe@gmail.com> — 2020-11-19T15:54:54Z

    >
> 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>

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

    Neil Chen <carpenter.nail.cz@gmail.com> — 2020-11-20T01:50:46Z

    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?

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

    David Zhang <david.zhang@highgo.ca> — 2020-11-24T18:36:33Z

    I verified the patch "v2-0001-Free-disk-space-for-dropped-relations-on-commit.patch" on master branch "0cc99327888840f2bf572303b68438e4caf62de9". It works for me. Below is my test procedure and results.

=== Before the patch ===
#1 from psql console 1, create table and index then insert enough data
postgres=# CREATE TABLE test_tbl ( a int, b text);
postgres=# CREATE INDEX idx_test_tbl on test_tbl (a);
postgres=# INSERT INTO test_tbl SELECT generate_series(1,80000000),'Hello world!';
postgres=# INSERT INTO test_tbl SELECT generate_series(1,80000000),'Hello world!';

#2 check files size 
david:12867$ du -h
12G	.

#3 from psql console 2, drop the index
postgres=# drop index idx_test_tbl;

#4 check files size in different ways,
david:12867$ du -h
7.8G	.
david:12867$ ls -l
...
-rw------- 1 david david          0 Nov 23 20:07 16402
...

$ lsof -nP | grep '(deleted)' |grep pgdata
...
postgres  25736                  david   45u      REG              259,2          0   12592758 /home/david/sandbox/postgres/pgdata/base/12867/16402 (deleted)
postgres  25736                  david   49u      REG              259,2 1073741824   12592798 /home/david/sandbox/postgres/pgdata/base/12867/16402.1 (deleted)
postgres  25736                  david   53u      REG              259,2 1073741824   12592739 /home/david/sandbox/postgres/pgdata/base/12867/16402.2 (deleted)
postgres  25736                  david   59u      REG              259,2  372604928   12592800 /home/david/sandbox/postgres/pgdata/base/12867/16402.3 (deleted)
...

The index relnode id "16402" displays size "0" from postgres database folder, but when using lsof to check, all 16402.x are still in used by a psql connection except 16402 is set to 0. Check it again after an hour, lsof shows the same results.

=== After the patch ===
Repeat step 1 ~ 4, lsof shows all the index relnode files (in this case, the index relnode id 16389) are removed within about 1 minute.
$ lsof -nP | grep '(deleted)' |grep pgdata
...
postgres  32707                  david   66u      REG              259,2          0   12592763 /home/david/sandbox/postgres/pgdata/base/12867/16389.1 (deleted)
postgres  32707                  david   70u      REG              259,2          0   12592823 /home/david/sandbox/postgres/pgdata/base/12867/16389.2 (deleted)
postgres  32707                  david   74u      REG              259,2          0   12592805 /home/david/sandbox/postgres/pgdata/base/12867/16389.3 (deleted)
...

One thing interesting for me is that, if the index is created after data records has been inserted, then lsof doesn't show this issue.

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

    Pavel Borisov <pashkin.elfe@gmail.com> — 2020-11-24T18:59:17Z

    The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:       tested, passed
Spec compliant:           not tested
Documentation:            not tested

Given we got two other reviews from Neil and David, I think I can finalize my own review and mark the patch as ready for committer if nobody has objections.
Thank you!

Pavel Borisov

The new status of this patch is: Ready for Committer

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-11-30T05:59:40Z

    On Wed, Nov 25, 2020 at 8:00 AM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> The new status of this patch is: Ready for Committer

Thanks!  One small thing bothered me about the last version of the
patch.  It tried to unlink when ENOENT had already been enountered by
open(2), so COMMIT of a DROP looks like:

openat(AT_FDCWD, "base/14208/16384", O_RDWR) = 54
ftruncate(54, 0)                        = 0
close(54)                               = 0
openat(AT_FDCWD, "base/14208/16384.1", O_RDWR) = -1 ENOENT
unlink("base/14208/16384.1")            = -1 ENOENT
openat(AT_FDCWD, "base/14208/16384_fsm", O_RDWR) = -1 ENOENT
unlink("base/14208/16384_fsm")          = -1 ENOENT
openat(AT_FDCWD, "base/14208/16384_vm", O_RDWR) = -1 ENOENT
unlink("base/14208/16384_vm")           = -1 ENOENT
openat(AT_FDCWD, "base/14208/16384_init", O_RDWR) = -1 ENOENT
unlink("base/14208/16384_init")         = -1 ENOENT

So I fixed that, by adding a return value to do_truncate() and
checking it.  That's the version I plan to commit tomorrow, unless
there are further comments or objections.  I've also attached a
version suitable for REL_11_STABLE and earlier branches (with a name
that cfbot should ignore), where things are slightly different.  In
those branches, the register_forget_request() logic is elsewhere.

While looking at trace output, I figured we should just use
truncate(2) on non-Windows, on the master branch only.  It's not like
it really makes much difference, but I don't see why we shouldn't
allow ourselves to use ancient standardised Unix syscalls when we can.
That'd get us down to just the following when committing a DROP:

truncate("base/14208/16396", 0)         = 0
truncate("base/14208/16396.1", 0)       = -1 ENOENT
truncate("base/14208/16396_fsm", 0)     = -1 ENOENT
truncate("base/14208/16396_vm", 0)      = -1 ENOENT
truncate("base/14208/16396_init", 0)    = -1 ENOENT

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-12-01T02:48:31Z

    On Mon, Nov 30, 2020 at 6:59 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Wed, Nov 25, 2020 at 8:00 AM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> > The new status of this patch is: Ready for Committer

> ...  That's the version I plan to commit tomorrow, unless
> there are further comments or objections.  ...

Done, and back-patched.

I thought a bit more about the fact that we fail to unlink
higher-numbered segments in certain error cases, potentially leaving
stray files behind.  As far as I can see, nothing we do in this
code-path is going to be a bullet-proof solution to that problem.  One
simple idea would be for the checkpointer to refuse to unlink segment
0 (thereby allowing the relfilenode to be recycled) until it has
scanned the parent directory for any related files that shouldn't be
there.

> While looking at trace output, I figured we should just use
> truncate(2) on non-Windows, on the master branch only.  It's not like
> it really makes much difference, but I don't see why we shouldn't
> allow ourselves to use ancient standardised Unix syscalls when we can.

Also pushed, but only to master.

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

    Michael Paquier <michael@paquier.xyz> — 2020-12-01T02:55:34Z

    On Mon, Nov 30, 2020 at 06:59:40PM +1300, Thomas Munro wrote:
> So I fixed that, by adding a return value to do_truncate() and
> checking it.  That's the version I plan to commit tomorrow, unless
> there are further comments or objections.  I've also attached a
> version suitable for REL_11_STABLE and earlier branches (with a name
> that cfbot should ignore), where things are slightly different.  In
> those branches, the register_forget_request() logic is elsewhere.

Hmm.  Sorry for arriving late at the party.  But is that really
something suitable for a backpatch?  Sure, it is not optimal to not
truncate all the segments when a transaction dropping a relation
commits, but this was not completely broken either.
--
Michael

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

    Thomas Munro <thomas.munro@gmail.com> — 2020-12-01T03:06:48Z

    On Tue, Dec 1, 2020 at 3:55 PM Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Nov 30, 2020 at 06:59:40PM +1300, Thomas Munro wrote:
> > So I fixed that, by adding a return value to do_truncate() and
> > checking it.  That's the version I plan to commit tomorrow, unless
> > there are further comments or objections.  I've also attached a
> > version suitable for REL_11_STABLE and earlier branches (with a name
> > that cfbot should ignore), where things are slightly different.  In
> > those branches, the register_forget_request() logic is elsewhere.
>
> Hmm.  Sorry for arriving late at the party.  But is that really
> something suitable for a backpatch?  Sure, it is not optimal to not
> truncate all the segments when a transaction dropping a relation
> commits, but this was not completely broken either.

I felt on balance it was a "bug", since it causes operational
difficulties for people and was clearly not our intended behaviour,
and I announced this intention 6 weeks ago.  Of course I'll be happy
to revert it from the back-branches if that's the consensus.  Any
other opinions?

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

    Michael Paquier <michael@paquier.xyz> — 2020-12-01T06:01:33Z

    On Tue, Dec 01, 2020 at 04:06:48PM +1300, Thomas Munro wrote:
> I felt on balance it was a "bug", since it causes operational
> difficulties for people and was clearly not our intended behaviour,
> and I announced this intention 6 weeks ago.

Oops, sorry for missing this discussion for such a long time :/

> Of course I'll be happy to revert it from the back-branches if
> that's the consensus.  Any > other opinions?

If there are no other opinions, I am also fine to rely on your
judgment.
--
Michael