Re: [v9.1] sepgsql - userspace access vector cache

> On Wed, Jul 20, 2011 at 9:47 AM, Yeb Havinga <yebhavinga(at)gmail(dot)com> wrote:
> > * I run SELECT sepgsql_restorecon(NULL) and saw comparable numbers in speed
> > gain and also backend process memory increase as indicated by KaiGai-san.
> > The vmsize without the patch increases from running restorecon 3864kB, with
> > the patch is increases 8160kB, a difference of ~5MB. Where this change comes
> > from is unclear to me. The avc_cache holds only 7 entries, and the avc
> > memory context stats indicate it's only at 8kB. Investigation into the
> > SECLABELOID syscache revealed that even when that cache was set to a
> > #buckets of 2, still after restorecon() the backend's vmsize increased about
> > the ~5MB more.
> >
> > * The size of SECLABELOID is currently set to 2048, which is equal to sizes
> > of the pg_proc and pg_attribute caches and hence makes sense. The initial
> > contents of pg_seclabel is 3346 entries. Just to get some idea what the
> > cachesize means for restorecon performance I tested some different values
> > (debugging was on). Until a size of 256, restorecon had comparable
> > performance. Small drop from ~415 to ~425 from 128 to 64. Cache of 32 had
> > ~435ms performance. Cache of 2 had 680ms. Without debugging symbols, I also
> > got a slightly better performance on the restorecon call with a 1024
> > syscache size. This might be something to tweak in later versions, when
> > there is more experience what this cache size means for performance on real
> > databases.
>
> Both of the above points make me real nervous, especially the second
> one. We already know that the cost of initializing the system caches
> is a significant chunk of backend startup overhead, and I believe that
> sizeof(Dllist) = 16 on 64-bit machine, so that means that 2048-entry
> catcache is going to require us to zero an additional 32kB of memory
> on every backend startup for a feature that most people won't use.
>
> http://archives.postgresql.org/pgsql-hackers/2010-11/msg01632.php
> http://archives.postgresql.org/pgsql-hackers/2010-11/msg01640.php
>
> As to the first point, the other big problem with adding a syscache
> here is that it could get really big. I've worried about that issue
> previously, and Yev's perplexity about where the memory is going is
> not too reassuring.
>
> I think there are two realistic ways forward here:
>
> 1. Bite the bullet and implement a system allowing catalog caches to
> be expanded on the fly at runtime. This would be nice because, aside
> from whatever value it may have for SE-PostgreSQL, it might allow us
> to shrink some of the other caches down and thereby speed up backend
> startup in general, with the confidence that if the session ends up
> running for a while we can expand them as necessary. In the
> particular case of SE-PostgreSQL, it might be nice to be able to set
> the initial cache size to 0, and only pay the cost of setting it up if
> we discover that the security label feature is in use; but maybe
> that's overly complex. On the downside, this addresses only the
> startup problem (zeroing previously unreferenced memory is fairly
> expensive) and not the runtime problem (using too much memory is bad).
> But perhaps there is some other solution to the second problem.
>
One question is why InitCatalogCache() should be invoked from InitPostgres()?
If we initialize syscache on starting up postmaster process, I think
all the syscache buckets will be ready on child process forks, and
unused syscache does not consume physical memory, even if security
label acquire 2048 of buckets.

> 2. Implement a custom cache tailored to the needs of SE-PostgreSQL
> within contrib/sepgsql. This is a bit tricky because you need some
> mechanism for receiving invalidation events.
> CacheRegisterSyscacheCallback() is the usual method, but that only
> lets you notice catcache invalidations, and here the whole point would
> be to avoid having a catcache in the first place. Possibly we could
> add a hook to PrepareForTupleInvalidation(); assuming that the hook
> engages only after the IsSystemRelation and IsToastRelation checks, it
> shouldn't cost that much. Doing it this way would (1) allow the
> sepgsql-specific cache to come into existence only when needed and (2)
> allow the sepgsl-specific cache to apply sepgsql-specific policies for
> controlling memory use. For example, it could cap the number of
> entries in the cache and age out any that are too old; or it could
> arrange to maintain a single copy of each label rather than a copy for
> each object.
>
I'm interested in this idea, however, not a work in this commit-fest.

So, I'll detach syscache part from the existing uavc patch (and shared
object's security label patch).

> I think that the uavc cache stuff may still be something we can think
> about committing for this 'fest (I haven't reviewed it yet), but IMHO
> the syscache parts need some more thought.
>
Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei <kohei(dot)kaigai(at)emea(dot)nec(dot)com>