On Tue, Jun 21, 2016 at 10:28:52PM +0200, Lukas Slebodnik wrote:
> On (21/06/16 22:08), Jakub Hrozek wrote:
> >On Tue, Jun 21, 2016 at 11:46:51AM +0200, Jakub Hrozek wrote:
> >> On Mon, Jun 20, 2016 at 06:57:55PM +0200, Jakub Hrozek wrote:
> >> > On Fri, Jun 10, 2016 at 01:33:49PM +0200, Jakub Hrozek wrote:
> >> > > On Fri, Jun 03, 2016 at 12:43:41PM +0200, Jakub Hrozek wrote:
> >> > > > Hi,
> >> > > >
> >> > > > the attached patchset improves SSSD by performance by adding
a second
> >> > > > ldb cache file that is opened with LDB_FLG_NOSYNC and
contains empeheral
> >> > > > data like timestamps. The reason is to avoid updating the
sync-cache in
> >> > > > case nothing in fact changed and SSSD is only refreshing the
cache to
> >> > > > add new timestamps. Especially for group lookups, this can
improve
> >> > > > performance a bit. The first lookup, or any where the actual
data
> >> > > > changes is still slow, though.
> >> > > >
> >> > > > I don't have a CI link yet, because I'm having
issues installing pyldb on
> >> > > > our test machines.
> >> > > >
> >> > > > The last patch doesn't touch sysdb, but the LDAP
provider. I hope it's
> >> > > > fine to include it in the same set, though.
> >> > > >
> >> > > > The full design document is at:
> >> > > >
https://fedorahosted.org/sssd/wiki/DesignDocs/OneFourteenPerformanceImpro...
> >> > >
> >> > > the attached patches fix one bug in the NSS responder related to
MPG
> >> > > domains and one issue with adding incomplete groups.
> >> >
> >> > I found another bug:
> >> > if you upgrade to this version from one that didn't have the
> >> > timestamp cache, sss_cache doesn't work until the cache is
> >> > invalidated.
> >> >
> >> > I'm not sure how to fix this yet, whether to create the timestamp
> >> > entries on upgrade to be safe or maybe invalidate all entries during
> >> > upgrade..but I would like to wait if there are any review comments
from
> >> > anybody before proceeding.
> >>
> >> Attached are patches rebased on today's origin/master.
> >
> >btw the integration test I wrote seems a bit flaky in CI:
> >
http://sssd-ci.duckdns.org/logs/job/45/68/rhel6/ci.html
> >although it just works for me locally. I wonder if it would be
> >acceptable to use the test only during patch review and fix it up later.
>
> You needn't remove test from patch.
> Just remove file with test.
> It should not start with "test_" or finish with "_test.py"
sleep(1) after the modification and before requesting the entry "solved"
it.
The attached patches also fix several Coverity issues and other review
comments Sumit found during his review.
CI:
http://sssd-ci.duckdns.org/logs/job/45/80/summary.html
Thank you for the patches. In my tests I've seen 5-10x faster lookups of
expired cached objects which didn't change on the server.
All my comments I discussed with Jakub on irc are included in the latest
patch set and CI in general passed as well
(the failure on
RHEL7 is in test_add_remove_membership_rfc2307_bis where I've seen
similar issues with other runs, maybe it is a timing issue as well, i.e.
we re-read the data from the server before 389ds was able to properly
update the related object?).
ACK
bye,
Sumit