nscd: Improve nscd.conf comments.
diff mbox series

Message ID 1d146c96-1c6f-c7d0-9a7e-bcaede3757f2@redhat.com
State New
Headers show
Series
  • nscd: Improve nscd.conf comments.
Related show

Commit Message

Carlos O'Donell Aug. 16, 2019, 3:22 p.m. UTC
Pinging this patch again now that 2.31 is out.

Now without format=flowed :-(

Comments

Florian Weimer Aug. 16, 2019, 3:45 p.m. UTC | #1
* 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
Carlos O'Donell Aug. 16, 2019, 4:11 p.m. UTC | #2
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?
Florian Weimer Aug. 16, 2019, 4:13 p.m. UTC | #3
* 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
Carlos O'Donell Aug. 16, 2019, 8:35 p.m. UTC | #4
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.
Florian Weimer Aug. 16, 2019, 8:53 p.m. UTC | #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
Carlos O'Donell Aug. 19, 2019, 7:48 p.m. UTC | #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.

Patch
diff mbox series

diff --git a/ChangeLog b/ChangeLog
index 23df9a3545..6309e7cac7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@ 
+2019-08-16  Carlos O'Donell  <carlos@redhat.com>
+
+	* nscd/nscd.conf: Add warning and comment about shared option.
+
 2019-08-16  Carlos O'Donell  <carlos@redhat.com>
 
 	* 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>
 #