URL:
https://github.com/SSSD/sssd/pull/289
Title: #289: Check the timestamp cache during the users/groups cleanup
fidencio commented:
"""
On Tue, May 30, 2017 at 4:44 PM, Jakub Hrozek <notifications(a)github.com>
wrote:
So the primary question I have is why not do what you're doing in
the
ldap_cleanup module in the sysdb search?
The main answer for this question is to avoid touching code that could
affect even more parts of the code (some of them which I'm not even aware
on how to test that those changes are not too intrusive).
I checked where are the sysdb_search_users and sysdb_search_groups
functions called and unfortunately some are in the critical paths which are
already quite slow, so adding the timestamp search before persistent cache
search might slow them even further..but then again, it might be cleaner to
add a sysdb_search_by_orig_dn which would always search only the persistent
cache.
Sorry Jakub, I'm not sure if I'm following you here. What would
sysdb_search_by_orig_dn() do? Would it do exactly what I'm doing in the
"LDAP_ID_CLEANUP: Check the ts_cache during the users/groups cleanup" patch
and just be used from ldap_cleanup()?
There are two reasons I propose this:
1. the sysdb_search_users/sysdb_search_groups interface is generic and
the caller should expect to return correct results regardless of whether
the filter contains a timestamp attribute or not
2. I don't think the sysdb.h interface should expose the timestamp
cache at all.
We don't disagree here, with any of the points. Just keep in mind that
sysdb_search_users() and sysdb_search_groups() do return correct results
based on the timestamp they have. The problem here is how often we update
the timestamp in the persistent cache.
I agree that adding the search by orig dn is also a bit of a hack, because
we already have too many search functions, but it seems like a more correct
solution.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<
https://github.com/SSSD/sssd/pull/289#issuecomment-304901159>, or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AAG4et-ew4vivfmrzG6N8zi...
.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/289#issuecomment-304994051