6:12 p.m.
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
Action: opened
PR body:
"""
When NSS receives SIGHUP signal it clears memory cache.
As a part of signal handling procedure negative cache
should be also cleared.
Resolves: https://github.com/SSSD/sssd/issues/4973
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
11:21 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
This PR missing flag / lock file implementation. Will be updated soon and then suitable
for review.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802063411
11:44 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
This PR missing flag / lock file implementation. Will be updated soon
and then suitable for review.
I'm not convinced it is really needed. And especially I wouldn't like to duplicate
this code, i.e. to have two lock files created/watched by single SIGHUP.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802099860
4:14 p.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Hi,
@pbrezina , do you remember the reason to have ["clearMemcache"
method](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_iface.... in
generic "sssd.service" interface instead of dedicated
["sssd.nss.MemoryCache"
interface](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_ifa...
Perhaps the idea was that monitor should treat all services as "generic"
service, but that's not the case anyway:
https://github.com/SSSD/sssd/blob/master/src/monitor/monitor.c#L1351
What happens if this method is called but service doesn't implement it? Is there
"stub" handler or just NULL in method table?
---
@elkoniu , I understand it's tempting to copy what is done wrt mem-cache (though I
would suggest to check if there is a need/opportunity to improve things a bit), but even
in this case:
- IIUC, your implementation of `nss_clear_negcache()` isn't correct (see comment
inline)
- please take a look at those functions:
https://github.com/SSSD/sssd/blob/master/src/responder/common/negcache.h#... (and
[existing
interface](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_ifa...)
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802304774
5:43 p.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
> This PR missing flag / lock file implementation. Will be updated
soon and then suitable for review.
I'm not convinced it is really needed. And especially I wouldn't like to
duplicate this code, i.e. to have two lock files created/watched by single SIGHUP.
Two lock files would be overkill. One is enough and can be used for both - memcache and
negcache. The purpose of this file is to identify SIGHUP received from sss_cache command
as only this one should trigger cache clearing.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802359827
5:53 p.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
```
<interface name="sssd.Responder.NegativeCache">
<annotation name="codegen.Name" value="resp_negcache"
/>
<annotation name="codegen.SyncCaller" value="false" />
<method name="ResetUsers" key="True" />
<method name="ResetGroups" key="True" />
</interface>
```
@alexey-tikhonov If I understand correct you suggest to update
`sssd.Responder.NegativeCache` with `RepopulatePermament` and then use this API instead of
generic `sssd.service` one (`"clearMemcache"` / `"clearNegCache"`)?
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802365243
5:54 p.m.
New subject: [sssd PR#5539][synchronized] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
7:43 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Two lock files would be overkill. One is enough and can be used for
both - memcache and negcache. The purpose of this file is to identify SIGHUP received from
sss_cache command as only this one should trigger cache clearing.
I don't think that's the purpose.
I think the purpose is to have a way to notify `sss_cache` that operation is finished.
But as I wrote, I'm not convinced this is really needed.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802807347
7:46 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
```
<interface name="sssd.Responder.NegativeCache">
<annotation name="codegen.Name" value="resp_negcache"
/>
<annotation name="codegen.SyncCaller" value="false"
/>
<method name="ResetUsers" key="True" />
<method name="ResetGroups" key="True" />
</interface>
```
@alexey-tikhonov If I understand correct you suggest to update
`sssd.Responder.NegativeCache` with `RepopulatePermament` and then use this API instead of
generic `sssd.service` one (`"clearMemcache"` / `"clearNegCache"`)?
No, I suggested to figure out if we can use responder's interface in monitor code and
if "yes" then just re-use existing ResetUsers/ResetGroups. You don't need to
touch permanent entries to resolve this issue.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802808502
7:50 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Two lock files would be overkill. One is enough and can be used for
both - memcache and negcache. The purpose of this file is to identify SIGHUP received from
sss_cache command as only this one should trigger cache clearing.
I'm not sure that's the main purpose.
I think the purpose also was to have a way to notify `sss_cache` that operation is
finished.
But as I wrote, I'm not convinced this is really needed.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802807347
9:04 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
The purpose of this file is to identify SIGHUP received from
sss_cache command as only this one should trigger cache clearing.
You are correct that this is one of purposes (besides notification that operation
completed).
99c99e5570
But I would question "why" the same signal SIGHUP was re-used both for log
rotation and mem-cache clearance. As a result, `sss_cache` triggers logs rotation. Is it
intentional?
Decoupling this would make this purpose obsolete.
And for the purpose of backward notification (if needed at all) sss_cache could monitor
mem-cache files itself.
This might be out of the scope of this ticket. But useful to keep in mind.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802857641
9:09 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
And please, if you think some comments are worth addition, feel free to do so but please
keep this as a separate patch. This would make `diff` easier to understand.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802860519
11:13 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
And please, if you think some comments are worth addition, feel free
to do so but please keep this as a separate patch. This would make `diff` easier to
understand.
I will split it into few commits when we agree that technical part is working correct.
But I would question "why" the same signal SIGHUP was
re-used both for log rotation and mem-cache clearance. As a result, sss_cache triggers
logs rotation. Is it intentional?
This is a good point, I think it may also contribute to the whole "slow files
provider" issue. But I think this should be addressed in separated PR / Issue.
And for the purpose of backward notification (if needed at all)
sss_cache could monitor mem-cache files itself.
I think approach with flag file was just simplest one. Also this way sss_cache needs to
care only about one file no mater how many cache files will be on SSSD side and how they
will be implemented.
You don't need to touch permanent entries to resolve this issue.
Here let me quote @sumit-bose [https://github.com/SSSD/sssd/issues/4973]
```
Hi,
you should check with @mzidek about his plans.
About the tricky part, the memory used by the in-memory tdb is not freed if individual
entries are removed so you have to close and open the tbd to get back the memory. Now you
should not forget to add the permanent entries again, but calling
sss_ncache_reset_repopulate_permanent() should be sufficient here.
HTH
bye,
Sumit
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802946884
11:13 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
And please, if you think some comments are worth addition, feel free
to do so but please keep this as a separate patch. This would make `diff` easier to
understand.
I will split it into few commits when we agree that technical part is working correct.
But I would question "why" the same signal SIGHUP was
re-used both for log rotation and mem-cache clearance. As a result, sss_cache triggers
logs rotation. Is it intentional?
This is a good point, I think it may also contribute to the whole "slow files
provider" issue. But I think this should be addressed in separated PR / Issue.
And for the purpose of backward notification (if needed at all)
sss_cache could monitor mem-cache files itself.
I think approach with flag file was just simplest one. Also this way sss_cache needs to
care only about one file no mater how many cache files will be on SSSD side and how they
will be implemented.
You don't need to touch permanent entries to resolve this issue.
Here let me quote @sumit-bose [https://github.com/SSSD/sssd/issues/4973]
```
Hi,
you should check with @mzidek about his plans.
About the tricky part, the memory used by the in-memory tdb is not freed if individual
entries are removed so you have
to close and open the tbd to get back the memory. Now you should not forget to add the
permanent entries again,
but calling sss_ncache_reset_repopulate_permanent() should be sufficient here.
HTH
bye,
Sumit
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802946884
11:14 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
And please, if you think some comments are worth addition, feel free
to do so but please keep this as a separate patch. This would make `diff` easier to
understand.
I will split it into few commits when we agree that technical part is working correct.
But I would question "why" the same signal SIGHUP was
re-used both for log rotation and mem-cache clearance. As a result, sss_cache triggers
logs rotation. Is it intentional?
This is a good point, I think it may also contribute to the whole "slow files
provider" issue. But I think this should be addressed in separated PR / Issue.
And for the purpose of backward notification (if needed at all)
sss_cache could monitor mem-cache files itself.
I think approach with flag file was just simplest one. Also this way sss_cache needs to
care only about one file no mater how many cache files will be on SSSD side and how they
will be implemented.
You don't need to touch permanent entries to resolve this issue.
Here let me quote @sumit-bose [https://github.com/SSSD/sssd/issues/4973]
```
Hi,
you should check with @mzidek about his plans.
About the tricky part, the memory used by the in-memory tdb is not freed if individual
entries are removed
so you have to close and open the tbd to get back the memory. Now you should not forget to
add
the permanent entries again, but calling sss_ncache_reset_repopulate_permanent() should be
sufficient here.
HTH
bye,
Sumit
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802946884
11:51 a.m.
New subject: [sssd PR#5539][comment] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
> But I would question "why" the same signal SIGHUP was
re-used both for log rotation and mem-cache clearance. As a result, sss_cache triggers
logs rotation. Is it intentional?
This is a good point, I think it may also contribute to the whole "slow files
provider" issue.
How?
> You don't need to touch permanent entries to resolve this
issue.
Here let me quote @sumit-bose
[https://github.com/[/issues/4973](https://github.com/SSSD/sssd/issues/4973)]
```
About the tricky part, the memory used by the in-memory tdb is not freed if individual
entries are removed
so you have to close and open the tbd to get back the memory. Now you should not forget
to add
the permanent entries again, but calling sss_ncache_reset_repopulate_permanent() should
be sufficient here.
```
If you don't touch permanent entries then you don't need to add it back again.
If you want to do a full cycle with close/open to partially resolve another ticket
mentioned (about mem leak), then this comment apply. But in your patch you don't do
this.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-802971608
8:24 p.m.
New subject: [sssd PR#5539][synchronized] [WIP] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: [WIP] NSS: Clear negative cache when SIGHUP received
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
8:24 p.m.
New subject: [sssd PR#5539][edited] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: NSS: Clear negative cache when SIGHUP received
Action: edited
Changed field: title
Original value:
"""
[WIP] NSS: Clear negative cache when SIGHUP received
"""
8:33 p.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
Bellow logs generated by calling `sudo sss_cache -E` on top of this PR:
```
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Monitor received SIGHUP
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Log rotate triggered for: sudo
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Log rotate triggered for: ifp
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Log rotate triggered for: pam
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Log rotate triggered for: nss
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): NSS negcache cleaning
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): NSS memcache cleaning
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): NSS enum_cache cleaning
(2021-03-23 1:29:13): [sssd] [monitor_hup] (0x0400): Log rotate triggered for: ldap.vm
```
```
(2021-03-23 1:29:13): [nss] [nss_clear_negcache] (0x0400): Clearing negative cache
non-permament entries
(2021-03-23 1:29:13): [nss] [sbus_issue_request_done] (0x0400):
sssd.service.clearNegcache: Success
(2021-03-23 1:29:13): [nss] [nss_clear_memcache] (0x0400): Clearing memory caches.
(2021-03-23 1:29:13): [nss] [sss_mmap_cache_init] (0x0100): Fast 'PASSWD' mmap
cache: timeout = 300, slots = 209712
(2021-03-23 1:29:13): [nss] [sss_mmap_cache_init] (0x0100): Fast 'GROUP' mmap
cache: timeout = 300, slots = 157288
(2021-03-23 1:29:13): [nss] [sss_mmap_cache_init] (0x0100): Fast 'INITGROUPS'
mmap cache: timeout = 300, slots = 262144
(2021-03-23 1:29:13): [nss] [sbus_issue_request_done] (0x0400):
sssd.service.clearMemcache: Success
(2021-03-23 1:29:13): [nss] [nss_clear_netgroup_hash_table] (0x0400): Invalidating
netgroup hash table
(2021-03-23 1:29:13): [nss] [sbus_issue_request_done] (0x0400):
sssd.service.clearEnumCache: Success
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-804517068
8:34 p.m.
New subject: [sssd PR#5539][+Waiting for review] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Waiting for review
6:02 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
pbrezina commented:
"""
@pbrezina , do you remember the reason to have
["clearMemcache"
method](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_iface.... in
generic "sssd.service" interface instead of dedicated
["sssd.nss.MemoryCache"
interface](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_ifa...
Legacy reasons. It looks like it can be moved to sssd.nss interface.
What happens if this method is called but service doesn't
implement it? Is there "stub" handler or just NULL in method table?
It will yield "unknown method" error in `sbus_method_handler`.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-806557130
4:21 p.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Thanks for the updates. This version definitely looks better and should do what is
functionally required.
What feels a little bit loose: patch relies on the order of method invocations in the
monitor to assure sss_cache is blocked until all operations are done. But I think proper
resolution should include decoupling of log rotation and cache reset, and since you said
you wouldn't like to do this within this PR, then proper comment would be enough.
As a more important note:
- instead of addition of a new method in "sssd.service" interface you could
check if it’s possible to reuse existing methods of
"sssd.Responder.NegativeCache"
- while not strictly necessary to be done within this PR, imo it also makes sense to do
the same with clearMemcache() method - either move it to “sssd.nss.MemoryCache” or just
get rid of it and reuse existing InvalidateAll*() (but the latter would require
verification that it does the same)
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-808515095
7:57 p.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
Thanks for the updates. This version definitely looks better and
should do what is functionally required.
What feels a little bit loose: patch relies on the order of method invocations in the
monitor to assure sss_cache is blocked until all operations are done. But I think proper
resolution should include decoupling of log rotation and cache reset, and since you said
you wouldn't like to do this within this PR, then proper comment would be enough.
I am almost sure that negative cache can be cleared without any blocker, also I did not
wanted to add another file blocker.
Existing blocker is part of legacy code. I would like to open separated PR to remove it,
split log rotate and cache cleaning and then handle whole thing using DBUS command send
from sss_cache directly, rather than sending SIGHUP to monitor.
I will add a comment in the code that I am sort-of reusing existing file blocker for new
function "just in case".
As a more important note:
* instead of addition of a new method in "sssd.service" interface you could
check if it’s possible to reuse existing methods of
"sssd.Responder.NegativeCache"
* while not strictly necessary to be done within this PR, imo it also makes sense to
do the same with clearMemcache() method - either move it to “sssd.nss.MemoryCache” or just
get rid of it and reuse existing InvalidateAll*() (but the latter would require
verification that it does the same)
I decided to add a new method to the code to be more consistent with existing
implementation. If it is possible I would like to move DBUS methods refactoring to
separated PR. Pavel mentioned [1] that there is a plan to get rid of monitor in favor of
systemd. I guess DBUS methods refactoring will be part of it too.
[1] https://github.com/SSSD/sssd/pull/5552#issuecomment-814906355
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-815370299
7:09 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
When I am trying to access `sssd.Responder.NegativeCache.ResetUsers` or
`sssd.Responder.NegativeCache.ResetGroups`
they are missing on the connector used by NSS by default:
```
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x2000): Received D-Bus method
sssd.Responder.NegativeCache.ResetUsers on /sssd
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x0020): Unknown interface!
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-815732250
7:10 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
elkoniu commented:
"""
instead of addition of a new method in "sssd.service"
interface you could check if it’s possible to reuse existing methods of
"sssd.Responder.NegativeCache"
When I am trying to access `sssd.Responder.NegativeCache.ResetUsers` or
`sssd.Responder.NegativeCache.ResetGroups`
they are missing on the connector:
```
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x2000): Received D-Bus method
sssd.Responder.NegativeCache.ResetUsers on /sssd
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x0020): Unknown interface!
```
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-815732250
9:01 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
When I am trying to access `sssd.Responder.NegativeCache.ResetUsers`
or `sssd.Responder.NegativeCache.ResetGroups`
they are missing on the connector:
```
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x2000): Received D-Bus method
sssd.Responder.NegativeCache.ResetUsers on /sssd
(2021-04-08 11:57:05): [nss] [sbus_method_handler] (0x0020): Unknown interface!
```
I guess this means responders do not register this interface on monitor's bus and
> @pbrezina , do you remember the reason to have
["clearMemcache"
method](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_iface.... in
generic "sssd.service" interface instead of dedicated
["sssd.nss.MemoryCache"
interface](https://github.com/SSSD/sssd/blob/master/src/sss_iface/sss_ifa...
Legacy reasons. It looks like it can be moved to sssd.nss interface.
we actually can't...
Well, ok, I will let other take a look.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-815850539
1:58 p.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Well, ok, I will let other take a look.
I mean, patch should do what is functionally required. But I'd like somebody else to
take a look in case we overlooked a more neat solution.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-816066519
5:39 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Looks good to me. I just wonder if it shouldn't reset all record
types - not only users and groups but also netgroups, services, etc?
From general point of view - probably yes.
But practically, AFAIK, the only use case for this reset - when sss_cache is invoked from
useradd/userdel.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-817701316
6:24 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
pbrezina commented:
"""
> Thanks for the updates. This version definitely looks better and
should do what is functionally required.
> What feels a little bit loose: patch relies on the order of method invocations in
the monitor to assure sss_cache is blocked until all operations are done. But I think
proper resolution should include decoupling of log rotation and cache reset, and since you
said you wouldn't like to do this within this PR, then proper comment would be
enough.
I am almost sure that negative cache can be cleared without any blocker, also I did not
wanted to add another file blocker.
Existing blocker is part of legacy code. I would like to open separated PR to remove it,
split log rotate and cache cleaning and then handle whole thing using DBUS command send
from sss_cache directly, rather than sending SIGHUP to monitor.
I will add a comment in the code that I am sort-of reusing existing file blocker for new
function "just in case".
> As a more important note:
> ```
> * instead of addition of a new method in "sssd.service" interface you
could check if it’s possible to reuse existing methods of
"sssd.Responder.NegativeCache"
>
> * while not strictly necessary to be done within this PR, imo it also makes sense to
do the same with clearMemcache() method - either move it to “sssd.nss.MemoryCache” or just
get rid of it and reuse existing InvalidateAll*() (but the latter would require
verification that it does the same)
> ```
Ideally, this kind of stuff should not be implemented as a method call but rather as a
dbus signal. For that however, we need to have single message bus.
I decided to add a new method to the code to be more consistent with existing
implementation. If it is possible I would like to move DBUS methods refactoring to
separated PR.
I'm fine with this.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-817730108
6:25 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
pbrezina commented:
"""
> Looks good to me. I just wonder if it shouldn't reset all
record types - not only users and groups but also netgroups, services, etc?
From general point of view - probably yes.
But practically, AFAIK, the only use case for this reset - when sss_cache is invoked from
useradd/userdel.
It's your and Pawels call. I'm not going to block these patches by it. Ack from
me.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-817730731
12:58 p.m.
New subject: [sssd PR#5539][synchronized] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: NSS: Clear negative cache when SIGHUP received
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
6:18 a.m.
New subject: [sssd PR#5539][-Waiting for review] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: -Waiting for review
6:18 a.m.
New subject: [sssd PR#5539][+Changes requested] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Changes requested
6:41 a.m.
New subject: [sssd PR#5539][synchronized] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: NSS: Clear negative cache when SIGHUP received
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
6:41 a.m.
New subject: [sssd PR#5539][-Changes requested] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: -Changes requested
6:41 a.m.
New subject: [sssd PR#5539][+Waiting for review] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Waiting for review
6:47 a.m.
New subject: [sssd PR#5539][-Waiting for review] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: -Waiting for review
6:47 a.m.
New subject: [sssd PR#5539][+Accepted] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Accepted
6:48 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
alexey-tikhonov commented:
"""
Thanks. ACK.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-818672826
7:43 a.m.
New subject: [sssd PR#5539][+Ready to push] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Ready to push
7:44 a.m.
New subject: [sssd PR#5539][comment] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5539
* `master`
* 7a4974c876ad5a8558dbd8897c7111a18aad47b1 - nss: Clear negative cache when SIGHUP
received
* 6195ac70bd4aa18fee30477af8918b7848ca939a - nss: Add negcache clearing sbus callback
* e69943594994e458508fcd443e3ba3865a643b1e - utils: Add description for CLEAR_MC_FLAG
define
* 4f3734274ce7c5ec84c4c244139cb35d2da40e78 - ncache: Fix misleading function comment
"""
See the full comment at https://github.com/SSSD/sssd/pull/5539#issuecomment-818705894
7:44 a.m.
New subject: [sssd PR#5539][-Accepted] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: -Accepted
7:44 a.m.
New subject: [sssd PR#5539][+Pushed] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: +Pushed
7:44 a.m.
New subject: [sssd PR#5539][-Ready to push] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Title: #5539: NSS: Clear negative cache when SIGHUP received
Label: -Ready to push
7:44 a.m.
New subject: [sssd PR#5539][closed] NSS: Clear negative cache when SIGHUP received
URL: https://github.com/SSSD/sssd/pull/5539
Author: elkoniu
Title: #5539: NSS: Clear negative cache when SIGHUP received
Action: closed
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5539/head:pr5539
git checkout pr5539
1106
days inactive
1133
days old
sssd-devel@lists.fedorahosted.org
44 comments
3 participants
participants (3)
-
alexey-tikhonov -
elkoniu
-
pbrezina