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

Simo Sorce simo at redhat.com
Wed Sep 11 14:16:03 UTC 2013


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.

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.

-- 
Simo Sorce * Red Hat, Inc * New York




More information about the sssd-devel mailing list