Re: [Pki-devel] [PATCH] pki-ftweedal-0015-Monitor-database-for-changes-to-LDAP-profiles.patch
On 10/1/2014 1:42 AM, Fraser Tweedale wrote:
On Mon, Sep 22, 2014 at 05:36:26PM +1000, Fraser Tweedale wrote:
> This is the first cut of the LDAP profile change monitoring. It
> depends on patches 0004..0009 and 0014
> (https://www.redhat.com/archives/pki-devel/2014-September/msg00052.html).
>
> To summarise the implementation: a separate thread carries out a
> persistent LDAP search and calls back into the ProfileSubsystem when
> changes occur. I haven't had much experience with persistent
> searches or multithreaded programming in Java, so eyeballs familiar
> with those areas are needed.
>
> I haven't yet tested with changes replicating between clones (a task
> for tomorrow) but I wanted to get the patch on list for feedback as
> early as possible.
>
Further to earlier email, I tested profile change replication
between clones and it is working well.
Some comments:
1. The MODDN operation can potentially happen if someone renames the
profile in LDAP directly, and that's easier to do with LDAP-based
profiles. I think we should handle this case too.
2. If the LDAP server is restarted the Monitor will stop working and
subsequent changes are not detected. The Monitor should automatically
reconnect in case the LDAP connection is interrupted.
3. Since the changesOnly is set to true, there's a possibility that
changes that happens after initial profile loading and before the
Monitor starts will not be detected. To avoid this problem, the
changesOnly should be false, and the initial profile loading should be
done in the Monitor as well.
4. The following assignment is redundant because the value is
overwritten by the next line:
String profileId = "<unknown>";
5. When receiving a MODIFY operation the Monitor calls forgetProfile().
It's probably redundant because readProfile() calls createProfile()
which calls forgetProfile() too.
6. Generally it's recommended to implement Runnable instead of extending
Thread. The Monitor can be merged into ProfileSubsystem.
I think #1, #2, #3 are important to fix because there is no mechanism to
refresh the profiles in memory other than by restarting the server, so
we cannot miss any changes in the database.
--
Endi S. Dewata