On Wed, Sep 10, 2014 at 5:35 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> (2014/09/10 12:31), Fujii Masao wrote:
>>
>> On Wed, Sep 10, 2014 at 12:15 PM, Etsuro Fujita
>> <fujita.etsuro@lab.ntt.co.jp> wrote:
>>>
>>> (2014/09/09 22:17), Fujii Masao wrote:
>>>>
>>>> Attached is the updated version of the patch.
>
>
>>> I took a quick review on the patch. It looks good to me,
>
>
>>> but one thing I'm
>>> concerned about is
>>>
>>> You wrote:
>>>>>>>
>>>>>>> The attached patch introduces the GIN index storage parameter
>>>>>>> "PENDING_LIST_CLEANUP_SIZE" which specifies the maximum size of
>>>>>>> GIN pending list. If it's not set, work_mem is used as that maximum
>>>>>>> size,
>>>>>>> instead. So this patch doesn't break the existing application which
>>>>>>> currently uses work_mem as the threshold of cleanup operation of
>>>>>>> the pending list. It has only not to set PENDING_LIST_CLEANUP_SIZE.
>>>
>>>
>>> As you mentioned, I think it's important to consider for the existing
>>> applications, but I'm wondering if it would be a bit confusing users to
>>> have
>>> two parameters,
>>
>>
>> Yep.
>>
>>> PENDING_LIST_CLEANUP_SIZE and work_mem, for this setting.
>>> Wouldn't it be easy-to-use to have only one parameter,
>>> PENDING_LIST_CLEANUP_SIZE? How about setting PENDING_LIST_CLEANUP_SIZE
>>> to
>>> work_mem as the default value when running the CREATE INDEX command?
>>
>>
>> That's an idea. But there might be some users who want to change
>> the cleanup size per session like they can do by setting work_mem,
>> and your idea would prevent them from doing that...
>
>
> Why not use ALTER INDEX ... SET (PENDING_LIST_CLEANUP_SIZE= ...)? Maybe I'm
> missing something, though.
It takes AccessExclusive lock and has an effect on every sessions
(not only specified session).
Regards,
--
Fujii Masao