[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On Tue, Aug 09, 2016 at 07:54:15AM +0200, Petr Cech wrote: > On 08/08/2016 04:20 PM, Petr Cech wrote: > > On 08/08/2016 04:09 PM, Lukas Slebodnik wrote: > > > On (08/08/16 15:36), Petr Cech wrote: > > > > On 08/08/2016 03:14 PM, Petr Cech wrote: > > > > > On 08/08/2016 02:40 PM, Lukas Slebodnik wrote: > > > > > > ehlo, > > > > > > > > > > > > The simple reproducer is to use getent passwd,group with > > > > > > non-existing > > > > > > entry. > > > > > > Without the patch you will see that "/var/lib/sss/mc/passwd" is > > > > > > opened > > > > > > twice. Onece with mode "rw" opened by sssd_nss and the 2nd time > > > > > > with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, > > > > > > getgrgid) > > > > > > > > > > > > LS > > > > > > > > > > Hello Lukas, > > > > > > > > > > it looks good to me. I am waiting for CI tests. > > > > > > > > Hi Lukas, > > > > > > > > CI failed: > > > > http://sssd-ci.duckdns.org/logs/job/51/05/summary.html > > > > > > > > On F24 during valgrind negcache-tests > > > > > > > > http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log > > > > > > > > > > > > I didn't find more descriptive log. Is there any? > > > > > > > Yes, > > > http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/negcache-tests.log > > > > > > http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/responder_common-tests.1122.valgrind.log > > > > > > > > > > > > Anyway, > > > attached is a patch which does not require changes in unit test. > > > > > > LS > > > > Great, > > > > this looks better to me. (LBTM?) > > > > I am waiting for CI tests. > > > > Regards > > > > CI nearly passed: > http://sssd-ci.duckdns.org/logs/job/51/07/summary.html > The failure is not connected. > > => ACK * master: 950716d2087446205c84f00b371f468d6ead1ec2 ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On 08/08/2016 04:20 PM, Petr Cech wrote: On 08/08/2016 04:09 PM, Lukas Slebodnik wrote: On (08/08/16 15:36), Petr Cech wrote: On 08/08/2016 03:14 PM, Petr Cech wrote: On 08/08/2016 02:40 PM, Lukas Slebodnik wrote: ehlo, The simple reproducer is to use getent passwd,group with non-existing entry. Without the patch you will see that "/var/lib/sss/mc/passwd" is opened twice. Onece with mode "rw" opened by sssd_nss and the 2nd time with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, getgrgid) LS Hello Lukas, it looks good to me. I am waiting for CI tests. Hi Lukas, CI failed: http://sssd-ci.duckdns.org/logs/job/51/05/summary.html On F24 during valgrind negcache-tests http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log I didn't find more descriptive log. Is there any? Yes, http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/negcache-tests.log http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/responder_common-tests.1122.valgrind.log Anyway, attached is a patch which does not require changes in unit test. LS Great, this looks better to me. (LBTM?) I am waiting for CI tests. Regards CI nearly passed: http://sssd-ci.duckdns.org/logs/job/51/07/summary.html The failure is not connected. => ACK Regards -- Petr^4 Čech ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On 08/08/2016 04:09 PM, Lukas Slebodnik wrote: On (08/08/16 15:36), Petr Cech wrote: On 08/08/2016 03:14 PM, Petr Cech wrote: On 08/08/2016 02:40 PM, Lukas Slebodnik wrote: ehlo, The simple reproducer is to use getent passwd,group with non-existing entry. Without the patch you will see that "/var/lib/sss/mc/passwd" is opened twice. Onece with mode "rw" opened by sssd_nss and the 2nd time with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, getgrgid) LS Hello Lukas, it looks good to me. I am waiting for CI tests. Hi Lukas, CI failed: http://sssd-ci.duckdns.org/logs/job/51/05/summary.html On F24 during valgrind negcache-tests http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log I didn't find more descriptive log. Is there any? Yes, http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/negcache-tests.log http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/responder_common-tests.1122.valgrind.log Anyway, attached is a patch which does not require changes in unit test. LS Great, this looks better to me. (LBTM?) I am waiting for CI tests. Regards -- Petr^4 Čech ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On (08/08/16 15:36), Petr Cech wrote:
>On 08/08/2016 03:14 PM, Petr Cech wrote:
>> On 08/08/2016 02:40 PM, Lukas Slebodnik wrote:
>> > ehlo,
>> >
>> > The simple reproducer is to use getent passwd,group with non-existing
>> > entry.
>> > Without the patch you will see that "/var/lib/sss/mc/passwd" is opened
>> > twice. Onece with mode "rw" opened by sssd_nss and the 2nd time
>> > with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, getgrgid)
>> >
>> > LS
>>
>> Hello Lukas,
>>
>> it looks good to me. I am waiting for CI tests.
>
>Hi Lukas,
>
>CI failed:
>http://sssd-ci.duckdns.org/logs/job/51/05/summary.html
>
>On F24 during valgrind negcache-tests
>
>http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log
>
>I didn't find more descriptive log. Is there any?
>
Yes,
http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/negcache-tests.log
http://sssd-ci.duckdns.org/logs/job/51/05/fedora24/ci-build-debug/src/tests/cwrap/responder_common-tests.1122.valgrind.log
Anyway,
attached is a patch which does not require changes in unit test.
LS
>From b38444238f9e01dce6919bdb0bf4da5a11142caf Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik
Date: Mon, 8 Aug 2016 13:55:52 +0200
Subject: [PATCH] NSS: Do not check local users with disabled
local_negative_timeout
sssd_nss can set different negative timeout for local users
and groups. However, checking whether user/group is local
is quite expensive operation. We can avoid such operations
if local_negative_timeout is not set.
This fix improve performance(40%) of lookup non-existing
entries in offline mode and with disabled local_negative_timeout.
sh$ cat pok.sh
for i in {1..1}; do
getent passwd -s sss temp$i
getent group -s sss temp$i
done
#without patch
sh $time /bin/bash pok.sh
real0m41.534s
user0m3.580s
sys 0m14.202s
#with patch
sh $time /bin/bash pok.sh
real0m26.686s
user0m3.292s
sys 0m13.165s
Resolves:
https://fedorahosted.org/sssd/ticket/3122
---
src/responder/common/negcache.c | 45 -
1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/src/responder/common/negcache.c b/src/responder/common/negcache.c
index
dfeb0d483e4db34cb2f25e1f82884611a707aabe..5b7ad69f432518be94b88e92e24265add722c852
100644
--- a/src/responder/common/negcache.c
+++ b/src/responder/common/negcache.c
@@ -143,7 +143,7 @@ done:
}
static int sss_ncache_set_str(struct sss_nc_ctx *ctx, char *str,
- bool permanent, bool is_local)
+ bool permanent, bool use_local_negative)
{
TDB_DATA key;
TDB_DATA data;
@@ -157,15 +157,16 @@ static int sss_ncache_set_str(struct sss_nc_ctx *ctx,
char *str,
if (permanent) {
timest = talloc_strdup(ctx, "0");
} else {
-if (is_local == true && ctx->local_timeout > 0) {
-timell = (unsigned long long int)time(NULL) + ctx->local_timeout;
+if (use_local_negative == true && ctx->local_timeout > ctx->timeout) {
+timell = ctx->local_timeout;
} else {
-if (ctx->timeout > 0) {
-timell = (unsigned long long int)time(NULL) + ctx->timeout;
-} else {
+/* EOK is tested in cwrap based unit test */
+if (ctx->timeout == 0) {
return EOK;
}
+timell = ctx->timeout;
}
+timell += (unsigned long long int)time(NULL);
timest = talloc_asprintf(ctx, "%llu", timell);
}
if (!timest) return ENOMEM;
@@ -457,7 +458,7 @@ int sss_ncache_check_cert(struct sss_nc_ctx *ctx, const
char *cert)
static int sss_ncache_set_user_int(struct sss_nc_ctx *ctx, bool permanent,
const char *domain, const char *name)
{
-bool is_local;
+bool use_local_negative = false;
char *str;
int ret;
@@ -466,8 +467,10 @@ static int sss_ncache_set_user_int(struct sss_nc_ctx *ctx,
bool permanent,
str = talloc_asprintf(ctx, "%s/%s/%s", NC_USER_PREFIX, domain, name);
if (!str) return ENOMEM;
-is_local = is_user_local_by_name(name);
-ret = sss_ncache_set_str(ctx, str, permanent, is_local);
+if (ctx->local_timeout > 0) {
+use_local_negative = is_user_local_by_name(name);
+}
+ret = sss_ncache_set_str(ctx, str, permanent, use_local_negative);
talloc_free(str);
return ret;
@@ -476,7 +479,7 @@ static int sss_ncache_set_user_int(struct sss_nc_ctx *ctx,
bool permanent,
static int sss_ncache_set_group_int(struct sss_nc_ctx *ctx, bool permanent,
const char *domain, const char *name)
{
-bool is_local;
+bool use_local_negative = false;
char *str;
int ret;
@@ -485,8 +488,10 @@ static int sss_ncache_set_group_int(struct sss_nc_ctx
*ctx, bool permanent,
str = talloc_asprintf(ctx, "%s/%s/%s", NC_GROUP_PREFIX,
[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On 08/08/2016 03:14 PM, Petr Cech wrote: On 08/08/2016 02:40 PM, Lukas Slebodnik wrote: ehlo, The simple reproducer is to use getent passwd,group with non-existing entry. Without the patch you will see that "/var/lib/sss/mc/passwd" is opened twice. Onece with mode "rw" opened by sssd_nss and the 2nd time with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, getgrgid) LS Hello Lukas, it looks good to me. I am waiting for CI tests. Hi Lukas, CI failed: http://sssd-ci.duckdns.org/logs/job/51/05/summary.html On F24 during valgrind negcache-tests http://sssd-ci.duckdns:.org/logs/job/51/05/fedora24/ci-build-debug/ci-make-check-valgrind.log I didn't find more descriptive log. Is there any? Regards -- Petr^4 Čech ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
[SSSD] Re: [PATCH] NSS: Do not check local users with disabled local_negative_timeout
On 08/08/2016 02:40 PM, Lukas Slebodnik wrote: ehlo, The simple reproducer is to use getent passwd,group with non-existing entry. Without the patch you will see that "/var/lib/sss/mc/passwd" is opened twice. Onece with mode "rw" opened by sssd_nss and the 2nd time with "r-" opened by sssd-client (getpwnam, getpwuid, getgrnam, getgrgid) LS Hello Lukas, it looks good to me. I am waiting for CI tests. Regards -- Petr^4 Čech ___ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
