Re: [PROPOSAL] Shared Ispell dictionaries

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PROPOSAL] Shared Ispell dictionaries
Date: 2019-01-20 23:43:59
Message-ID: 95aea959-97a1-2ded-c881-eede4e5a1f0c@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/20/19 11:21 PM, Andres Freund wrote:
> On 2019-01-20 23:15:35 +0100, Tomas Vondra wrote:
>> On 1/17/19 3:15 PM, Arthur Zakirov wrote:
>>> I attached files of new version of the patch, I applied your tweaks.
>>>
>>>> XXX All dictionaries, but only when there's invalid dictionary?
>>>
>>> I've made a little optimization. I introduced hashvalue into
>>> TSDictionaryCacheEntry. Now released only DSM of altered or dropped
>>> dictionaries.
>>>
>>>>   > /* XXX not really a pointer, so the name is misleading */
>>>>
>>>> I think we don't need DictPointerData struct anymore, because only
>>>> ts_dict_shmem_release function needs it (see comments above) and we only
>>>> need it to hash search. I'll move all fields of DictPointerData to
>>>> TsearchDictKey struct.
>>>
>>> I was wrong, DictInitData also needs DictPointerData. I didn't remove
>>> DictPointerData, I renamed it to DictEntryData. Hope that it is a more
>>> appropriate name.
>>>
>>
>> Thanks. I've reviewed v17 today and I haven't discovered any new issues
>> so far. If everything goes fine and no one protests, I plan to get it
>> committed over the next week or so.
>
> There doesn't seem to be any docs about what's needed to be able to take
> advantage of shared dicts, and how to prevent them from permanently
> taking up a significant share of memory.
>

Yeah, those are good points. I agree the comments might be clearer, but
essentially ispell dictionaries are shared and everything else is not.

As for the memory consumption / unloading dicts - I agree that's
something we need to address. There used to be a way to specify memory
limit and ability to unload dictionaries explicitly, but both features
have been ditched. The assumption was that UNLOAD would be introduced
later, but that does not seem to have happened.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-01-20 23:48:13 Allowing extensions to supply operator-/function-specific info
Previous Message Haribabu Kommi 2019-01-20 23:32:37 Re: Pluggable Storage - Andres's take