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.

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 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.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company