[PATCHES v2 BUG 13865] Properly track the size of talloc objects

[Christof Schmitt]

On Fri, Apr 05, 2019 at 03:15:17PM -0700, Jeremy Allison wrote:
> On Fri, Apr 05, 2019 at 03:09:09PM -0700, Jeremy Allison via samba-technical wrote:
> > On Fri, Apr 05, 2019 at 02:21:55PM -0700, Christof Schmitt via samba-technical wrote:
> > > Ping. Does anybody want to review? The pipeline now passes.
> > 
> > OK, LGTM and RB+. Feel free to push to master.
> > Thanks for helping me track down the issues
> > with it originally.
> > 
> > But we also must think about the effect this
> > will have on the size settings for the global
> > cache.
> > 
> > Right now we do:
> > 
> > smbd_memcache_ctx = memcache_init(NULL, lp_max_stat_cache_size()*1024);
> > memcache_set_global(smbd_memcache());
> > 
> > where max stat cache size = 256.
> > 
> > This means we have a 256k total size global memcache.
> > 
> > Previously we're only storing an 8 byte
> > pointer (on a 64-bit platform) per talloc
> > object going into the global cache.
> > 
> > The global memcache is being used for
> > talloc_caches in:
> > 
> > source3/auth/token_util.c (SINGLETON_CACHE_TALLOC)
> > source3/lib/username.c (GETPWNAM_CACHE)
> > source3/locking/share_mode_lock.c (SHARE_MODE_LOCK_CACHE)
> > source3/passdb/pdb_interface.c (PDB_GETPWSID_CACHE)
> > source3/smbd/vfs.c (GETWD_CACHE)
> > 
> > The possible increases in size accounting
> > mostly come from the GETWD_CACHE (current
> > pathnames up to 4K bytes) and the
> > SHARE_MODE_LOCK_CACHE (there are
> > 3 pathnames here, plus 3 pointers
> > to possibly large data structures,
> > guess at 4K each).
> > 
> > That's probably about 16K on average
> > extra storeage per SHARE_MODE_LOCK_CACHE
> > and one 4K GETWD_CACHE entry to account
> > for instead of one 8 byte pointer.
> > 
> > With 100 open files that's approx 200K
> > more we need to store.
> > 
> > My gut feeling is along with this patch
> > you need a supplementary patch to change
> > "max stat cache size = 8*1024"
> > giving us a 8Mb global memcache
> > max size per smbd to avoid cache
> > evictions.
> > 
> > With 1000 concurrent connections each
> > with 100 open files that's an max of
> > 8gb of memory for caching, which seems
> > reasonable to me (NB for interested followers
> > this memory is *not* allocated on startup,
> > that's just the max size the cache can
> > grow to given use over time).
> 
> One more point. In case someone thinks
> setting "max stat cache size = 8192"
> will cause us to use more memory, just
> remember right now without Christof's
> patch we're not accounting for *any*
> of the talloc memory being stored in
> the cache, beyond an 8 byte pointer
> per object. Which means currently
> the memcache is essentially unlimited
> in the memory it can cache :-).
> 
> I just want us to set a new reasonable
> default :-).

Anything we choose is a guess, since every workload is different. On a
quick test to open multiple files with smbclient and then checking size
allocated under 'struct memcache' in smbcontrol pool-usage, shows that
each open file increases the memory usage by something like 300 to 700
bytes.

So that 16k for each share mode seems high, although that might increase
with concurrent access to the same file. 4k for each getwd cache also
seems high, that seems to mainly depend on the length of the path name.

Rounding the share mode size up to 1024 bytes and the getwd length to
1024, and assuming one getwd cache for each open file (probably also too
high), that would yield 100 * 1024 + 100 * 1024 = 200k

That would be more in line with doubling the state cache size from 256
to 512 (kB).

In the end i am fine with any number, just trying to get a feeling for
the actual space used here.

Christof