[SSSD] [PATCH] DB: Update sss_domain_info with new updated data

Jakub Hrozek jhrozek at redhat.com
Wed Aug 28 16:05:54 UTC 2013


On Wed, Aug 28, 2013 at 05:58:40PM +0200, Sumit Bose wrote:
> 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

Pushed to master.



More information about the sssd-devel mailing list