Subject: Re: [PROPOSAL] Shared Ispell dictionaries
To: Andres Freund <andres@anarazel.de>
Cc: Arthur Zakirov <a.zakirov@postgrespro.ru>,
 Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 Pavel Stehule <pavel.stehule@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <27296.1522078068@sss.pgh.pa.us>
 <20180327121954.GA12726@zakirov.localdomain>
 <CA+TgmoaR_fVd_u60mA-PHO=J1u0PmdVGOt9WaxZ1UYuGq276oQ@mail.gmail.com>
 <20180516113631.GA29544@zakirov.localdomain>
 <20180614084015.GA12451@zakirov.localdomain>
 <20181001092204.GA5071@zakirov.localdomain>
 <68aaaff6-0efe-c14b-7aee-fb110bb97f69@2ndquadrant.com>
 <337f7a55-58b8-4fcc-a933-5e7799fa6882@postgrespro.ru>
 <e0a25563-cbb1-7089-8c25-44668cfb031b@postgrespro.ru>
 <42b1ded6-fd44-5975-77e4-72ada1f134a2@2ndquadrant.com>
 <20190120222121.ehistvcr4wsu2su5@alap3.anarazel.de>
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Message-ID: <95aea959-97a1-2ded-c881-eede4e5a1f0c@2ndquadrant.com>
Date: Mon, 21 Jan 2019 00:43:59 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <20190120222121.ehistvcr4wsu2su5@alap3.anarazel.de>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

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