Re: PATCH: optimized DROP of multiple tables within a transaction

From: Tomas Vondra <tv(at)fuzzy(dot)cz>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: optimized DROP of multiple tables within a transaction
Date: 2013-01-08 20:56:41
Message-ID: 50EC8809.4090002@fuzzy.cz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8.1.2013 03:47, Shigeru Hanada wrote:
>>> * naming of DROP_RELATIONS_BSEARCH_LIMIT (or off-by-one bug?)
>>> IIUC bsearch is used when # of relations to be dropped is *more* than
>>> the value of DROP_RELATIONS_BSEARCH_LIMIT (10). IMO this behavior is
>>> not what the macro name implies; I thought that bsearch is used for 10
>>> relations. Besides, the word "LIMIT" is used as *upper limit* in
>>> other macros. How about MIN_DROP_REL[ATION]S_BSEARCH or
>>> DROP_REL[ATION]S_LINEAR_SEARCH_LIMIT?
>>> # using RELS instead of RELATIONS would be better to shorten the name
>>>
>>
>> I've changed the name to DROP_RELS_BSEARCH_THRESHOLD. I believe this
>> naming is consistent with options like "geqo_threshold" - when the
>> number of relations reaches the specified value, the bsearch is used.
>>
>> I've also increased the value from 10 to 20, in accordance with the
>> previous discussion.
>
> New name sounds good to me, but the #define says that the value is 15.
> Should it be fixed to 20?

Ah, yes, please increase it to 20. I've increased it from 10 to 15
first, but I think 20 is nearer the optimal value and I forgot to change
it in the code.

>
>>> * +1 for Alvaro's suggestion about avoiding palloc traffic by starting
>>> with 8 elements or so.
>>>
>>
>> Done.
>
> Not yet. Initial size of srels array is still 1 element.

I've checked the patch and I see there 'maxrels = 8' - or do you mean
something else?

Tomas

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2013-01-08 21:00:10 Re: [PATCH 2/5] Make relpathbackend return a statically result instead of palloc()'ing it
Previous Message Tom Lane 2013-01-08 20:55:24 Re: Re: [PATCH 1/5] Centralize Assert* macros into c.h so its common between backend/frontend