From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: Speedup of relation deletes during recovery |
Date: | 2018-06-18 18:13:47 |
Message-ID: | 20180618181347.o4cowslr2qobhtdc@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2018-06-19 03:06:54 +0900, Fujii Masao wrote:
> On Sat, Jun 16, 2018 at 3:28 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > Hi,
> >
> > On 2018-06-15 10:45:04 -0700, Andres Freund wrote:
> >> > +
> >> > + srels = palloc(sizeof(SMgrRelation) * ndelrels);
> >> > for (i = 0; i < ndelrels; i++)
> >> > - {
> >> > - SMgrRelation srel = smgropen(delrels[i], InvalidBackendId);
> >> > + srels[i] = smgropen(delrels[i], InvalidBackendId);
> >> >
> >> > - smgrdounlink(srel, false);
> >> > - smgrclose(srel);
> >> > - }
> >> > + smgrdounlinkall(srels, ndelrels, false);
> >> > +
> >> > + for (i = 0; i < ndelrels; i++)
> >> > + smgrclose(srels[i]);
> >> > + pfree(srels);
> >
> > Hm. This will probably cause another complexity issue. If we just
> > smgropen() the relation will be unowned. Which means smgrclose() will
> > call remove_from_unowned_list(), which is O(open-relations). Which means
> > this change afaict creates a new O(ndrels^2) behaviour?
> >
> > It seems like the easiest fix for that would be to have a local
> > SMgrRelationData in that loop, that we assign the relations to? That's
> > a bit hacky, but afaict should work?
>
> The easier (but tricky) fix for that is to call smgrclose() for
> each SMgrRelation in the reverse order. That is, we should do
>
> for (i = ndelrels - 1; i >= 0; i--)
> smgrclose(srels[i]);
>
> instead of
>
> for (i = 0; i < ndelrels; i++)
> smgrclose(srels[i]);
We could do that - but add_to_unowned_list() is actually a bottleneck in
other places during recovery too. We pretty much never (outside of
dropping relations / databases) close opened relations during recovery -
which is obviously problematic since nearly all of the entries are
unowned. I've come to the conclusion that we should have a global
variable that just disables adding anything to the global lists.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Dent John | 2018-06-18 18:38:13 | Re: Query Rewrite for Materialized Views (Postgres Extension) |
Previous Message | Fujii Masao | 2018-06-18 18:06:54 | Re: Speedup of relation deletes during recovery |