MIME-Version: 1.0
Date: Tue, 25 Sep 2012 16:10:15 -0500
Message-ID: <CAK3OfOiQtCCL70pEn=a6qjYoN8o45gq561phuL8cEjtni626iw@mail.gmail.com>
Subject: iprop can block for extended periods due to UPDATE_BUSY
From: Nico Williams <nico@cryptonector.com>
To: krb5-bugs@mit.edu
Content-Type: text/plain; charset=UTF-8
Precedence: list
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu
Content-Length: 1873

Currently kadmind allows slaves to poll for updates as often as they
like, but not within 10s of the last update.  This means that iprop will
appear to fail to synchronize the KDC at any site whose master KDC
processes at least one write transaction every 10 seconds consistently.

The original intention must have been to throttle iprop clients (slave
KDCs) that poll too often.  But UPDATE_BUSY as implemented is not that,
and implementing a throttle would be difficult (requires keeping state
in a table) and mostly useless (admins can manage their poll timers just
fine without a throttle in kadmind).

The simplest fix would be to remove all semblance of UPDATE_BUSY
handling in kadmind:

diff --git a/src/lib/kdb/kdb_log.c b/src/lib/kdb/kdb_log.c
index dc994dd..b800fa6 100644
--- a/src/lib/kdb/kdb_log.c
+++ b/src/lib/kdb/kdb_log.c
@@ -726,10 +726,9 @@ ulog_get_entries(krb5_context context,
/* input - krb5 lib config */
     XDR                 xdrs;
     kdb_ent_header_t    *indx_log;
     kdb_incr_update_t   *upd;
-    uint_t              indx, count, tdiff;
+    uint_t              indx, count;
     uint32_t            sno;
     krb5_error_code     retval;
-    struct timeval      timestamp;
     kdb_log_context     *log_ctx;
     kdb_hlog_t          *ulog = NULL;
     uint32_t            ulogentries;
@@ -750,15 +749,6 @@ ulog_get_entries(krb5_context context,
/* input - krb5 lib config */
         return (KRB5_LOG_CORRUPT);
     }

-    gettimeofday(&timestamp, NULL);
-
-    tdiff = timestamp.tv_sec - ulog->kdb_last_time.seconds;
-    if (tdiff <= ULOG_IDLE_TIME) {
-        ulog_handle->ret = UPDATE_BUSY;
-        (void) ulog_lock(context, KRB5_LOCKMODE_UNLOCK);
-        return (0);
-    }
-
     /*
      * We need to lock out other processes here, such as kadmin.local,
      * since we are looking at the last_sno and looking up updates.  So