[PATCH] nscd: Improve nscd.conf comments.

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] nscd: Improve nscd.conf comments.

Carlos O'Donell-6
Pinging this patch again now that 2.31 is out.

Now without format=flowed :-(

--
Cheers,
Carlos.

8< --- 8< --- 8<
From 552bf14ec9fc0dc90bc726a475a36f464e0012a1 Mon Sep 17 00:00:00 2001
From: Carlos O'Donell <[hidden email]>
Date: Wed, 20 Mar 2019 22:11:32 -0400
Subject: [PATCH] nscd: Improve nscd.conf comments.

This change adds a warning to nscd.conf about running multiple caching
services together and that it may lead to unexpected behaviours. Also we
add a note that enabling the 'shared' option will cause cache hit rates
to be misreported (a side effect of the implementation).
---
 ChangeLog      | 4 ++++
 nscd/nscd.conf | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 23df9a3545..6309e7cac7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2019-08-16  Carlos O'Donell  <[hidden email]>
+
+ * nscd/nscd.conf: Add warning and comment about shared option.
+
 2019-08-16  Carlos O'Donell  <[hidden email]>
 
  * nss/nsswitch.conf: Expand comments, and simplify defaults.
diff --git a/nscd/nscd.conf b/nscd/nscd.conf
index 39b875912d..ec8a8c1622 100644
--- a/nscd/nscd.conf
+++ b/nscd/nscd.conf
@@ -3,6 +3,9 @@
 #
 # An example Name Service Cache config file.  This file is needed by nscd.
 #
+# WARNING: Running nscd with a secondary caching service like sssd may lead to
+#          unexpected behaviour, especially with how long entries are cached.
+#
 # Legal entries are:
 #
 # logfile <file>
@@ -23,6 +26,9 @@
 # check-files <service> <yes|no>
 # persistent <service> <yes|no>
 # shared <service> <yes|no>
+# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup
+#      with the help of the client, but these lookups will not be
+#      counted as cache hits i.e. 'nscd -g' may show '0%'.
 # max-db-size <service> <number bytes>
 # auto-propagate <service> <yes|no>
 #
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Florian Weimer-5
* Carlos O'Donell:

> +# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup
> +#      with the help of the client, but these lookups will not be
> +#      counted as cache hits i.e. 'nscd -g' may show '0%'.

The “with the help of the client” part is quite confusing.  I think you
should say that for the shared case, nscd does not observe any cache
hits at all.  It's not that the client performs the lookup on behalf of
nscd or something like that.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Carlos O'Donell-6
On 8/16/19 11:45 AM, Florian Weimer wrote:

> * Carlos O'Donell:
>
>> +# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup
>> +#      with the help of the client, but these lookups will not be
>> +#      counted as cache hits i.e. 'nscd -g' may show '0%'.
>
> The “with the help of the client” part is quite confusing.  I think you
> should say that for the shared case, nscd does not observe any cache
> hits at all.  It's not that the client performs the lookup on behalf of
> nscd or something like that.

I guess you're right the client does the lookup itself, not on behalf of
nscd, it gets to do it's own lookup for iteslf.

IIUC you are suggesting that the implementation detail is not relevant
to the note that nscd will not see the cache hits? That's a good point
and simplifies this NOTE.

How about something like this:
~~~
NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
      but those lookups will not be counted as cache hits i.e. 'nscd -g'
      may show '0%'.
~~~

That avoids implementation details but documents the current behaviour
that you get when you enable the shared cache and expect lookups to
contribute to cache hit statistics.

Does that text look OK?

--
Cheers,
Carlos.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Florian Weimer-5
* Carlos O'Donell:

> How about something like this:
> ~~~
> NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
>       but those lookups will not be counted as cache hits i.e. 'nscd -g'
>       may show '0%'.
> ~~~
>
> That avoids implementation details but documents the current behaviour
> that you get when you enable the shared cache and expect lookups to
> contribute to cache hit statistics.
>
> Does that text look OK?

Yes, much better.

Thanks,
Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Carlos O'Donell-6
On 8/16/19 12:13 PM, Florian Weimer wrote:

> * Carlos O'Donell:
>
>> How about something like this:
>> ~~~
>> NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
>>       but those lookups will not be counted as cache hits i.e. 'nscd -g'
>>       may show '0%'.
>> ~~~
>>
>> That avoids implementation details but documents the current behaviour
>> that you get when you enable the shared cache and expect lookups to
>> contribute to cache hit statistics.
>>
>> Does that text look OK?
>
> Yes, much better.

v2 with rewritten text.

--
Cheers,
Carlos.

8< --- 8< --- 8<
From 63ed92cbfb1b2ae0011dcf05543e26f2a1425f6f Mon Sep 17 00:00:00 2001
From: Carlos O'Donell <[hidden email]>
Date: Wed, 20 Mar 2019 22:11:32 -0400
Subject: [PATCH] nscd: Improve nscd.conf comments.

This change adds a warning to nscd.conf about running multiple caching
services together and that it may lead to unexpected behaviours. Also we
add a note that enabling the 'shared' option will cause cache hit rates
to be misreported (a side effect of the implementation).

v2
- Rewrite comment to avoid implementation details.
---
 ChangeLog      | 4 ++++
 nscd/nscd.conf | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 23df9a3545..6309e7cac7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2019-08-16  Carlos O'Donell  <[hidden email]>
+
+ * nscd/nscd.conf: Add warning and comment about shared option.
+
 2019-08-16  Carlos O'Donell  <[hidden email]>
 
  * nss/nsswitch.conf: Expand comments, and simplify defaults.
diff --git a/nscd/nscd.conf b/nscd/nscd.conf
index 39b875912d..487ffe461d 100644
--- a/nscd/nscd.conf
+++ b/nscd/nscd.conf
@@ -3,6 +3,9 @@
 #
 # An example Name Service Cache config file.  This file is needed by nscd.
 #
+# WARNING: Running nscd with a secondary caching service like sssd may lead to
+#          unexpected behaviour, especially with how long entries are cached.
+#
 # Legal entries are:
 #
 # logfile <file>
@@ -23,6 +26,9 @@
 # check-files <service> <yes|no>
 # persistent <service> <yes|no>
 # shared <service> <yes|no>
+# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
+#      but those lookups will not be counted as cache hits
+#      i.e. 'nscd -g' may show '0%'.
 # max-db-size <service> <number bytes>
 # auto-propagate <service> <yes|no>
 #
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Florian Weimer-5
* Carlos O'Donell:

> diff --git a/nscd/nscd.conf b/nscd/nscd.conf
> index 39b875912d..487ffe461d 100644
> --- a/nscd/nscd.conf
> +++ b/nscd/nscd.conf
> @@ -3,6 +3,9 @@
>  #
>  # An example Name Service Cache config file.  This file is needed by nscd.
>  #
> +# WARNING: Running nscd with a secondary caching service like sssd may lead to
> +#          unexpected behaviour, especially with how long entries are cached.
> +#
>  # Legal entries are:
>  #
>  # logfile <file>
> @@ -23,6 +26,9 @@
>  # check-files <service> <yes|no>
>  # persistent <service> <yes|no>
>  # shared <service> <yes|no>
> +# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
> +#      but those lookups will not be counted as cache hits
> +#      i.e. 'nscd -g' may show '0%'.
>  # max-db-size <service> <number bytes>
>  # auto-propagate <service> <yes|no>
>  #

Looks good to me, thanks.

Florian
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] nscd: Improve nscd.conf comments.

Carlos O'Donell-6
On 8/16/19 4:53 PM, Florian Weimer wrote:

> * Carlos O'Donell:
>
>> diff --git a/nscd/nscd.conf b/nscd/nscd.conf
>> index 39b875912d..487ffe461d 100644
>> --- a/nscd/nscd.conf
>> +++ b/nscd/nscd.conf
>> @@ -3,6 +3,9 @@
>>  #
>>  # An example Name Service Cache config file.  This file is needed by nscd.
>>  #
>> +# WARNING: Running nscd with a secondary caching service like sssd may lead to
>> +#          unexpected behaviour, especially with how long entries are cached.
>> +#
>>  # Legal entries are:
>>  #
>>  # logfile <file>
>> @@ -23,6 +26,9 @@
>>  # check-files <service> <yes|no>
>>  # persistent <service> <yes|no>
>>  # shared <service> <yes|no>
>> +# NOTE: Setting 'shared' to a value of 'yes' will accelerate the lookup,
>> +#      but those lookups will not be counted as cache hits
>> +#      i.e. 'nscd -g' may show '0%'.
>>  # max-db-size <service> <number bytes>
>>  # auto-propagate <service> <yes|no>
>>  #
>
> Looks good to me, thanks.

Pushed. Thanks.

I'm rebasing Fedora to use this file now.

That harmonizes all of our downstream changes for nscd.conf
and nsswitch.conf.

--
Cheers,
Carlos.