On (28/07/16 13:56), 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?
It was just
a proposal.
We can still decide to use more conservative way and do not rely on
modifyTimestamp.
We can also used your 3rd proposal. Add option for enabling
detection for modifyTimestamp which would be disabled by default for generic
LDAP servers.
The question is what is the best/the safest for downstream
and what is a performance enhancement. I did't do any measurements.
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).
LS