On Thu, Jul 28, 2016 at 01:56:50PM +0200, Jakub Hrozek wrote: > On Thu, Jul 28, 2016 at 01:33:32PM +0200, Lukas Slebodnik wrote: > > On (28/07/16 12:06), thierry bordaz wrote: > > >On 07/28/2016 09:39 AM, Jakub Hrozek wrote: > > >> On Wed, Jul 27, 2016 at 04:09:07PM +0200, thierry bordaz wrote: > > >> > > > >> > On 07/27/2016 03:36 PM, Jakub Hrozek wrote: > > >> > > On Wed, Jul 27, 2016 at 02:55:37PM +0200, thierry bordaz wrote: > > >> > > > On 07/27/2016 01:56 PM, Jakub Hrozek wrote: > > >> > > > > On Wed, Jul 27, 2016 at 01:03:59PM +0200, Jakub Hrozek wrote: > > >> > > > > > On Wed, Jul 27, 2016 at 12:22:46PM +0200, Lukas Slebodnik > > >> > > > > > wrote: > > >> > > > > > > On (27/07/16 12:08), Jakub Hrozek wrote: > > >> > > > > > > > On Wed, Jul 27, 2016 at 12:02:24PM +0200, Jakub Hrozek > > >> > > > > > > > wrote: > > >> > > > > > > > > On Wed, Jul 27, 2016 at 11:54:16AM +0200, Lukas > > >> > > > > > > > > Slebodnik wrote: > > >> > > > > > > > > > ehlo, > > >> > > > > > > > > > > > >> > > > > > > > > > attached patch fixes acces denied after activating > > >> > > > > > > > > > user in 389ds. > > >> > > > > > > > > > Jakub had some comments/ideas in ticket but I think > > >> > > > > > > > > > it's better to discuss > > >> > > > > > > > > > about virtual attributes and timestamp cache on > > >> > > > > > > > > > mailing list. > > >> > > > > > > > > Yes, so the comment I have is that while this works, it > > >> > > > > > > > > might break some > > >> > > > > > > > > strange LDAP servers. > > >> > > > > > > > > > > >> > > > > > > > > We use modifyTimestamp as a 'positive' indicator that > > >> > > > > > > > > the entry has not > > >> > > > > > > > > changed -- if the modifyTimestamp didn't change, we > > >> > > > > > > > > consider the cached > > >> > > > > > > > > entry the same as what is on the server and only bump > > >> > > > > > > > > the timestamp > > >> > > > > > > > > cache. If the timestamp is different, we do a > > >> > > > > > > > > deep-comparison of cached > > >> > > > > > > > > attribute values with what is on the LDAP server and > > >> > > > > > > > > write the sysdb > > >> > > > > > > > > cache entry only if the attributes differ. > > >> > > > > > > > > > > >> > > > > > > > > I was wondering if we can use the modifyTimestamp at > > >> > > > > > > > > all, then, because > > >> > > > > > > > > even if it's the same, we might want to check the > > >> > > > > > > > > attributes to see if > > >> > > > > > > > > some of the values are different because some of the > > >> > > > > > > > > attributes might be > > >> > > > > > > > > this operational/virtual attribute.. > > >> > > > > > > > Sorry, sent too soon. > > >> > > > > > > > > > >> > > > > > > > I think the questions are -- 1) can we enumerate the > > >> > > > > > > > virtual attributes? > > >> > > > > > > That might be a question for 389-ds developers. > > >> > > > > > > But it's very likely it will be different on other LDAP > > >> > > > > > > servers. > > >> > > > > > > > > >> > > > > > > > 2) Would different LDAP servers have different virtual > > >> > > > > > > > attributes. > > >> > > > > > > > > > >> > > > > > > > For 2) maybe a possible solution might be to set a > > >> > > > > > > > non-existing > > >> > > > > > > > modifyTimestamp attribute value, but I would consider that > > >> > > > > > > > only a > > >> > > > > > > > kludge, we shouldn't break existing setups.. > > >> > > > > > > I am not satisfied with this POC solution either. > > >> > > > > > > > > >> > > > > > > So should we remove usage of modifyTimestamp for detecting > > >> > > > > > > changes? > > >> > > > > > I would prefer to ask the DS developers before removing it > > >> > > > > > completely. > > >> > > > > > > > >> > > > > > At least for large groups it might take a long time to compare > > >> > > > > > all attribute > > >> > > > > > values and IIRC we don't depend on any virtual attributes for > > >> > > > > > groups. Maybe > > >> > > > > > we could parametrize that part of the code and enable the fast > > >> > > > > > way with > > >> > > > > > modifyTimestamps for 'known' server types, that is for setups > > >> > > > > > with AD and > > >> > > > > > IPA providers. > > >> > > > > > > > >> > > > > > For users, there is typically not as many attributes so we > > >> > > > > > should be > > >> > > > > > fine deep-comparing all attributes. > > >> > > > > I'm adding Thierry (so please reply-to-all to keep him in the > > >> > > > > thread). > > >> > > > > > > >> > > > > Thierry, in the latest sssd version we tried to add a performance > > >> > > > > improvement related to how we store SSSD entries in the cache. > > >> > > > > The short > > >> > > > > version is that we store the modifyTimestamp attribute in the > > >> > > > > cache and > > >> > > > > when we fetch an entry, we compare the entry modifyTimestamp > > >> > > > > with what > > >> > > > > is on the server. When the two are the same, we say that the > > >> > > > > entry did > > >> > > > > not change and don't update the cache. > > >> > > > > > > >> > > > > This works fine for most attributes, but not for attributes like > > >> > > > > nsAccountLock which do not change modifyTimestamp when they are > > >> > > > > modified. So when an entry was already cached but then > > >> > > > > nsAccountLock > > >> > > > > changed, we treated the entry as the same and never read the new > > >> > > > > nsAccountLock value. > > >> > > > > > > >> > > > > To fix this, I think we have several options: > > >> > > > > 1) special-case the nsAccountLock. This seems a bit > > >> > > > > dangerous, > > >> > > > > because I'm not sure we can say that some other attribute > > >> > > > > we are > > >> > > > > interested in behaves the same as nsAccountLock. > > >> > > > > 2) drop the modifyTimestamp optimization completely. Then > > >> > > > > we fall > > >> > > > > back to comparing the attribute values, which might work, > > >> > > > > but for > > >> > > > > huge objects like groups with thousands of members, this > > >> > > > > might be > > >> > > > > too expensive. > > >> > > > > 3) only use the modifyTimestamp optimization for cases > > >> > > > > where we know > > >> > > > > we don't read any virtual attributes. > > >> > > > > > > >> > > > > And my question is -- can we, in general, know if the > > >> > > > > modifyTimestamp > > >> > > > > way of detecting changes is realiable for all LDAP servers? Or > > >> > > > > do you > > >> > > > > think it should only be used for cases where we know we are not > > >> > > > > interested in any virtual attributes (that would mostly be > > >> > > > > storing > > >> > > > > groups from servers where we know exactly what is on the server > > >> > > > > side, > > >> > > > > like IPA or AD). > > >> > > > Hello, > > >> > > > > > >> > > > Relying on modifytimestamp looks a good idea. Any MOD/MODRDN will > > >> > > > update it, > > >> > > > except I think it is unchanged when updating some password policy > > >> > > > attributes. > > >> > > > > > >> > > > Regarding virtual attribute, the only one I know in IPA is > > >> > > > nsaccountlock. > > >> > > > nsaccountlock is an operational attribute (you need to request it > > >> > > > to see it) > > >> > > > and is also a virtual attribute BUT only for 'staged' and > > >> > > > 'deleted' users. > > >> > > > It is a stored attribute for regular users and we should update > > >> > > > modifytimestamp when it is set. > > >> > > > > > >> > > > thanks > > >> > > > thierry > > >> > > OK, in that case it seems like we can special-case it. But do you > > >> > > know > > >> > > about any other attributes in any other LDAP servers? > > >> > Any LDAP server following standard should provide modifytimestamp that > > >> > reflect the last update of the entry. Now virtual attribute values may > > >> > be > > >> > "attached" to the entry and its value change without modification of > > >> > modifytimestamp. > > >> > For 389-ds and IPA it is fine as virtual value of nsaccountlock is > > >> > changed > > >> > only when the DN change. > > >> > For others LDAP servers I suppose it exists the same ability to define > > >> > service providers that return virtual attribute values. The difficulty > > >> > is > > >> > that the schema may not give any hint if the retrieved attributes > > >> > values > > >> > were stored or computed and consequently trust modifytimestamp to know > > >> > if > > >> > the values changed or not. > > >> > For example in ODSEE, memberof is a virtual attribute. > > >> Thank you, for the explanation Thierry. > > >> > > >> Then to be on the safe side I propose: > > >> 1) We add an (probably undocumented?) flag that says whether to use > > >> modifyTimestamp to detect entry changes or not > > >> 2) for the generic LDAP provider we always really compare the > > >> attribute values, in other words the option would be set to > > >> false. If there is anyone with performance issues with a generic > > >> setup, we tell them to flip the option. > > >> 3) For the IPA and AD providers, we set this option to true and use > > >> the modifyTimestamp value to detect changes > > >> 4) We special case nsAccountLock > > >> > > >> Lukas, do you agree? > > >Hi Jakub, > > > > > >digging further into the server, it appears that a DS plugin 'acctpolicy' > > >updates an entry without changing the mofidytimestamp. > > >The updated attribute is 'lastlogintime' (by default) but i think can be > > >any > > >attribute configured in the entry account policy. > > >I need to do further tests to confirm this. > > > > > > > IMHO, the problems with nsAccountLock just revealed the fact that > > it might happen that the attribute modifyTimestamp/whenChanged needn't be > > a reliable way how to determine change in entry for any LDAP Server. > > > > We improved the performance but there might be other corner cases with > > other LDAP servers. > > > > Jakub proosed in 2) that we should always really compare the the attibutes > > from sssd cache and from LDAP search. But it would mean that we would not > > improve a performance for generic LDAP providers. > > We would still avoid the cache writes, "only" after comparing the > attribute values. So essentially by using the modifyTimestamp we save a > single LDB base search and a number of attribute-value comparisons, > depending on how large the object is. > > > > > We cannot generally detect virtual attributes for any LDAP server. > > What about adding an option where user could list virtual attributes. > > It would be a kind of proposed solution 4) but it would not be a > > special case for nsAccountLock but for more attributes which could be > > changed > > in configuration. > > OK, so your proposal is to keep the more aggressive cache optimization? > I think I'm mostly afraid about the more exotic LDAP servers, like IBM > Tivoli or Novell eDirectory which I already know from experience don't > follow the established standards closely. I guess I'm fine with that, we > can always flip the default back. Worst case for people who start using > 1.14, the admin can always define modifyTimestamp to some non-existing > attribute and force the attribute value comparison check. > > Yes, we can make the list configurable. We also need to be > very careful about printing the reason for (not) updating the cache to > the admin (#3060).
OK, so after some discussion with Simo on IRC, there is a different proposal - let responder control the optimization level, so that PAM responder would always trigger a full cache write, or at least deep-compare the attributes and NSS responder would rely on modifyTimestamp. But that's a larger fix, so for short-term fix I propose to only use modifyTimestamp for group objects and always compare attributes for users. Then later, as another patch we can let the responder send a flag to control the optimization (would probably be done as a flag for a sysdb transaction). If you agree, I would file a ticket for the second part and you can instead write a patch to disable modifyTimestamp checks for users. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
