[SSSD] [PATCH] mmap_cache: Use two chains for hash collision.

Michal Židek mzidek at redhat.com
Wed Sep 11 14:30:48 UTC 2013


On 09/11/2013 04:16 PM, Simo Sorce wrote:
> On Wed, 2013-09-11 at 15:58 +0200, Lukas Slebodnik wrote:
>> On (11/09/13 09:39), Simo Sorce wrote:
>>> On Wed, 2013-09-11 at 12:24 +0200, Lukas Slebodnik wrote:
>>>> ehlo,
>>>>
>>>> Two patches are attached.
>>>>
>>>> The 1st patch reverts commit 4662725ffef62b3b2502481438effa7c8fef9f80.
>>>> "mmap_cache: Skip records which doesn't have same hash"
>>>>   --next patch lokks nicer with recerted commit 4662725ffe
>>>>
>>>>
>>>> The 2nd patch changes behaviour of record chaining.
>>>>
>>>> LS
>>>
>>> NACK, you cannot just change the record structure w/o bumping the whole
>>> mmap MAJOR version number.
>>>
>>> Aside:
>>>
>>> If we are not I am thinking whether we should do some other
>>> optimizations to keep the size down.
>>>
>>> For example, hashes are generally relatively small quantities as we do
>>> not allow huge tables. If we can limit tables size to a max of 65k
>>> elements we could change the hash type from uint32_t to uint16_t, and
>>> end up using the same size we are using now.
>>> We already limit the total mmap size to 4G as rel_prt_t is a uint32_t,
>>> and in practice we'll never allow maps with a very large size as it eats
>>> into the memory available to 32 bit processes.
>>> So perhaps 65k for the hash tables is an acceptable limit. (I think we
>>> currently have a 100k limit which is probably a little bit overblown as
>>> a default anyway.
>>>
>>> Simo.
>>>
>> I would rather reduce "uint64_t expire" into "uint32_t expire_offset"
>>
>> expire_offset will be time since creation of mmap cache.
>> and uint64_t "creation_time" will be stored in "struct sss_mc_ctx"
>
> Uhmm, I do agree that 4billion seconds offset is probably ok, but then
> it means the information is split in 2 different places, and this will
> make clients more complicated, as clients will have to keep track of the
> creation time + expiration offset now ...
>
>> Do you agree?
>
> I am not 100% convinced yet, although I do agree that it would save
> quite some space.
>
>> Do I need to increment SSS_MC_MAJOR_VNO or only SSS_MC_MINOR_VNO?
>
> Major, you are completely changing the structure, an older client cannot
> be allowed to use the new map unless it explicitly know the layout
> changed.

Well, if we already have to change the major version, I would go with
the two 32 bit next1 and nex2 members. If we really want to keep the
current size of the record (btw. why? We have to change the major ver.
anyway) then I like the two 16 bit pointers next1 and next2 more than
the 32 bit time offset (the memcache is just another cache layer, so it
does not have to be "gigantic"... 16 bit pointers are just fine IMO).

>
> Note that this means that until a long lived process is restarted after
> the upgrade that changes this structure it will fail to use the cache
> consistently (as it should, or it would just read garbage).
>
> Simo.
>




More information about the sssd-devel mailing list