On Tue, Aug 27, 2013 at 11:18:23AM +0200, Jakub Hrozek wrote: > On Mon, Aug 26, 2013 at 04:34:14PM +0200, Sumit Bose wrote: > > On Thu, Aug 22, 2013 at 11:50:27AM +0200, Jakub Hrozek wrote: > > > On Mon, Aug 19, 2013 at 08:14:03AM -0400, Simo Sorce wrote: > > > > On Mon, 2013-08-19 at 11:33 +0200, Sumit Bose wrote: > > > > > On Sun, Aug 18, 2013 at 10:26:46PM +0200, Jakub Hrozek wrote: > > > > > > There seems to be a logic bug in sysdb_master_domain_add_info(). We > > > > > > first update the domain data with sysdb search results and then > > > > > > update > > > > > > sysdb. I think it should be the other way around. > > > > > > > > > > iirc the idea was to read the sysdb entry first to make sure we have > > > > > the > > > > > latest data from sysdb. Because of the asynchronous nature of the > > > > > request it might be possible that another request already updated the > > > > > sysdb entry. To avoid unneeded writes we try to compare the new data > > > > > with the latest data we know. This is led by the assumption that > > > > > reading from sysdb is a lot cheaper then writing to it. > > > > > > > > How often does this operation happen ? > > > > Simo. > > > > > > Never until now because it didn't work :-) But if there's a cheap way to > > > optimize, then why not. I think the code should just make it clear > > > what's happening. > > > > > > Attached is a new patch that explains why we call the domain update also > > > before the sysdb update and adds another domain update after the sysdb > > > update to make sure the sss_domain_info structure has the latest data. > > > > > > After a bit more thinking I think the first version of the patch is the > > better one. The main reason is that a change in the value will only > > happen once for an IPA server and never for AD. When running > > ipa-adtrust-install the IPA domain will get its SID and flat name and a > > running SSSD will detect the change and update his structures. Since it > > is a rare event I think it's acceptable to let parallel running task to > > update the domain entry twice because in the general case the call to > > sysdb_master_domain_update() is not needed because the server data and > > the data in the struct will be the same. > > OK. I don't really mind either way but attached is the first version > again. > > > > > Additionally both callers set the realm to NULL, i.e. it is not updated > > at all because it is derived from the configuration for the master > > domains. Would you mind to just remove this parameter? > > Yes, that's in a separate patch so that the changes can be seen > logically. >
ACK bye, Sumit _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
