[Pki-devel] [PATCH] pki-ftweedal-0015-Monitor-database-for-changes-to-LDAP-profiles.patch

[Endi Sukma Dewata]

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