On Mon, Dec 3, 2012 at 8:52 PM, Emmanuel Lécharny <elecharny@gmail.com> wrote:
Le 12/3/12 3:59 PM, Kiran Ayyagari a écrit :
> On Mon, Dec 3, 2012 at 6:58 PM, Emmanuel Lécharny <elecharny@gmail.com>wrote:
>> Hi guys,
>> reviewing the current replication implementation, I have a few proposals
>> to improve some overhead.
>> First, let's describe the way it works :
>> 1) LdapServer starts the consumers
>>   for each of them, we create a new thread that init the consumer, and
>> start it (consumer.start())
>> 2) We try to connect to the provider, in a loop. When we get connected,
>> we call startSync() that will start the replication
>> 3) In startSync, we handle two different cases :
>>  - refreshOnly uses a thread that loop until stopped (which occurs only
>> when we shutdown the LdapServer)
>> <note>
>> Here, we have an issue : we don't handle the disconnection from a remote
>> provider.
>> </note>
>> no we do, note that the connectionClosed() is called by the
> LdapNetworkConnection based on MINA's sessionClosed() callback

Ah, right. This is covered later in my mail, but I wrote it after having
written this sentence, and forgot to fix that.
>> <note>
>> There is also no need to create a new thread, we can use the one that
>> has been created while starting the consumer
>> </note>
>> no, we don't create a new thread, the thread is created once and it stays
> alive till the server stops

We have the thread created in  LdapServer :

    public void startReplicationConsumers() throws Exception
        if ( replConsumers != null )
            for ( final ReplicationConsumer consumer : replConsumers )
                Runnable consumerTask = new Runnable()
                    public void run()

and the RefresherThread thread.

What I say is that we can merge those both thread.
> When the provider get disconnected, the way it works is a bit subtil :
> the connectionClosed() method is called, and restart the consumer.
> Except that it starts it in another thread, because we may have exited
> from the original thread. (the connectionClosed() call is done using a
> thread from the MINA executor, and if the disconnection has occured, the
> consumer is not anymore handled by the thread that started it). Unless
> I'm missing something.
> <note>
> Here, I would suggest to *never* get out of the thread that started the
> consumer, except if we explicitely require so. When the
> connectionClosed() method is called, we simply exit from the loop we are
> in, and go to sleep before retrying to reconnect later.
> </note>
> again, we don't create/start a new thread, the same thread is used

We do if we use the RefresherThread.

right, I was referring to the other thread we create to handle refreshOnly mode (cause each consumer has to run in a separate thread anyway)

Emmanuel Lécharny

Kiran Ayyagari