Re: PENDING_LIST_CLEANUP_SIZE - maximum size of GIN pending list Re: HEAD seems to generate larger WAL regarding GIN index

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Sawada Masahiko <sawada(dot)mshk(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Jesper Krogh <jesper(at)krogh(dot)cc>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PENDING_LIST_CLEANUP_SIZE - maximum size of GIN pending list Re: HEAD seems to generate larger WAL regarding GIN index
Date: 2014-11-10 17:31:57
Message-ID: CAHGQGwFFFeseo4iw18My2Krj-V=Oo0bY9VQgE+99rWOO1JRTfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita
<fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> (2014/11/06 23:38), Fujii Masao wrote:
>>
>> On Tue, Nov 4, 2014 at 12:04 PM, Etsuro Fujita
>> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>>
>>> IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
>>> appropriate to set min to some positive value. And ISTM that the idea of
>>> using the min value of work_mem is not so bad.
>>
>>
>> OK. I changed the min value to 64kB.
>>
>>> *** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable
>>> class="parameter">name</
>>> --- 356,372 ----
>>> </listitem>
>>> </varlistentry>
>>> </variablelist>
>>> + <variablelist>
>>> + <varlistentry>
>>> + <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
>>>
>>> The above is still in uppercse.
>>
>>
>> Fixed.
>>
>> Attached is the updated version of the patch. Thanks for the review!
>
>
> Thanks for the updating the patch!
>
> The patch looks good to me except for the following point:

Thanks for the review again!

>
> *** a/src/backend/access/gin/ginfast.c
> --- b/src/backend/access/gin/ginfast.c
> ***************
> *** 25,30 ****
> --- 25,32 ----
> #include "utils/memutils.h"
> #include "utils/rel.h"
>
> + /* GUC parameter */
> + int pending_list_cleanup_size = 0;
>
> I think we need to initialize the GUC to boot_val, 4096 in this case.

No, IIUC basically the variable for GUC doesn't need to be initialized
to its default value. OTOH, it's also harmless to initialize it to the default.
I like the current code a bit because we don't need to change the initial
value again when we decide to change the default value of GUC.
I have no strong opinion about this, though.

Regards,

--
Fujii Masao

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2014-11-10 17:33:47 Re: Teaching pg_dump to use NOT VALID constraints
Previous Message Alvaro Herrera 2014-11-10 17:28:30 Re: Proposal: Log inability to lock pages during vacuum