On 05/02/2014 02:52 PM, Simo Sorce wrote:
On Thu, 2014-05-01 at 16:22 -0400, Nathaniel McCallum wrote:
On Tue, 2014-03-11 at 11:09 -0400, Simo Sorce wrote:
On Tue, 2014-03-11 at 16:05 +0200, Alexander Bokovoy wrote:
On Tue, 11 Mar 2014, Jan Pazdziora wrote:
On Mon, Feb 24, 2014 at 02:26:27PM -0500, Nathaniel McCallum wrote:
Before this patch, ipa-kdb would load global configuration on startup
and never update it. This means that if global configuration is changed,
the KDC never receives the new configuration until it is restarted.

This patch enables caching of the global configuration with a timeout of
60 seconds.

https://fedorahosted.org/freeipa/ticket/4153
>From 7daeae56671d7b3049b0341aad66c96877431bbe Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <npmccal...@redhat.com>
Date: Mon, 24 Feb 2014 14:19:13 -0500
Subject: [PATCH] Periodically refresh global ipa-kdb configuration

Before this patch, ipa-kdb would load global configuration on startup and
never update it. This means that if global configuration is changed, the
KDC never receives the new configuration until it is restarted.

This patch enables caching of the global configuration with a timeout of
60 seconds.

https://fedorahosted.org/freeipa/ticket/4153
I have only read the code and it looks sane, so depending on how much
you consider my word about code-reading worth, ack.

However, my gut feeling is that my preferred way of handling the issue
(without knowing much about the background of the ticket) would
probably be a HUP signal handler or something similar, rather than
polling for current values with the value timeout. This patch
introduces small nondeterminism to the behaviour when the usage of the
new values cannot really be enfoced by the admin (without the daemon
restart).
Thing is, we need the update to happen when other, non-root process
makes the changes -- in our case when IPA server running under httpd
privileges performs series of MS-RPC calls towards smbd which lead to
creating certain LDAP objects. httpd couldn't send SIGHUP to a process
not owned by httpd's effective user (non-root).
Yes but this is not really the way to go.

The proper fix is to use syncrepl/persistent search to get a
notification of when we need to reload the configuration.

On the patch itself I have a NACK due to this syntax used in various
places: function()->attribute

don't. do. that. ever.

assign whatever come from the function to a local variable and *check*
it is not null, *then* use it.
Attached patch fixes the NACK issue, but does not address the question
of the larger approach. Do we need to take a different approach? If so,
what is it?
LGTM
I might prefer slightly more explicit names for those temp vars, but
that's not a big deal.

As for the approach, moving to something like syncrepl would be a good
idea. As it would allow us to avoid useless polling and changes would be
applaied as soon as they become available as the syncrepl agreement
would push them to our client. It does mean we need a way to check that
there aren't pending changes coming down the syncrepl pipe, but we can
do that synchronously at every entry point in the KDB. After all we do
not need to care about a change until the KDC needs something from the
KDB.

Simo.

Do we need a ticket for that then?

--
Thank you,
Dmitri Pal

Sr. Engineering Manager IdM portfolio
Red Hat, Inc.

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to