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

Jakub Hrozek jhrozek at redhat.com
Thu Aug 22 09:50:27 UTC 2013


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.
-------------- next part --------------
>From b303e25a4a891aebb61e04e97b40dd61c7191f01 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sat, 17 Aug 2013 09:12:03 +0200
Subject: [PATCH] DB: Update sss_domain_info with new updated data

The code didn't reload the sss_domain_info structure in memory after
changing the domain data on disk, which caused the SSSD to keep using
stale data.

Also adds a comment that explains why we call the update twice.
---
 src/db/sysdb_subdomains.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/db/sysdb_subdomains.c b/src/db/sysdb_subdomains.c
index 58df470182ea6ecbb5b48e288ab6a3d3bcec18ba..e6eab46f320f04d50097dd5d1bfc3141e6b29be9 100644
--- a/src/db/sysdb_subdomains.c
+++ b/src/db/sysdb_subdomains.c
@@ -264,6 +264,11 @@ errno_t sysdb_master_domain_add_info(struct sss_domain_info *domain,
         return ENOMEM;
     }
 
+    /* First, update the domain object to make sure we're using the same
+     * data as the data on disk. Then we might find out that just the
+     * structure in memory was outdated and won't need the write
+     * to cache at all
+     */
     ret = sysdb_master_domain_update(domain);
     if (ret != EOK) {
         goto done;
@@ -350,6 +355,14 @@ errno_t sysdb_master_domain_add_info(struct sss_domain_info *domain,
         goto done;
     }
 
+    /* Because the contents of domain on disk changed, we need to refresh
+     * the structure as well
+     */
+    ret = sysdb_master_domain_update(domain);
+    if (ret != EOK) {
+        goto done;
+    }
+
     ret = EOK;
 
 done:
-- 
1.8.3.1



More information about the sssd-devel mailing list