mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Niklas Therning <nik...@trillian.se>
Subject Re: Switching codecs in runtime (Was: Removing synchronization on a ProtocolEncoder and a ProtocolDecoder)
Date Fri, 09 Jun 2006 19:19:20 GMT
Niklas Therning wrote:
> Trustin Lee wrote:
>> Hi Niklas,
>>
>> On 4/26/06, Niklas Therning <niklas@trillian.se> wrote:
>>> Trustin Lee wrote:
>>>> On 3/27/06, Niklas Therning <niklas@trillian.se> wrote:
>>>>
>>>>> In protocols like SMTP when there are simple line-based commands
>>>>> intermixed with raw data (mail data) there are also great opportunities
>>>>> for optimization if you write your own codec filter. I've
>>> implemented my
>>>>> own DecoderFilter which can operate in "data mode". When not in data
>>>>> mode the filter will act more or less like an ordinary ProtocolDecoder,
>>>>> copying the received buffer to an autoexpanding buffer, decode SMTP
>>>>> commands and pass them on to the next filter. When in data mode however
>>>>> the filter will simply forward the buffers as they are received without
>>>>> any copying (in most cases).
>>>>>
>>>>> I guess this could be achieved with the MINA codec package but not
>>>>> without some tweaking and not as efficiently. Please let me know if I'm
>>>>> wrong.
>>>>>
>>>>> I wouldn't mind adding this filter to MINA if anyone is interested.
>>>>
>>>>
>>>> It would be nice if we can generalize this behavior.  We could then
>>> switch
>>>> arbitrary set of filters in runtime fairly easily.  WDYT?
>>>>
>>>> Trustin
>>> I think DIRMINA-201 solves the efficiency issue I was referring to. The
>>> only thing that would have to change to make ProtocolCodecFilter suit my
>>> needs is that SimpleProtocolDecoderOut shouldn't queue messages but
>>> rather forward them to the nextFilter right away as they are written.
>>> Then my IoHandler would be able to instruct my Decoder instance to
>>> change its state and decode differently depending on what the previous
>>> message was.
>>
>> If we directly call the IoHandler.messageReceived() when we call
>> ProtocolDecoderOut.write(), any exceptions raised by the handler can
>> interrupt  the decoding process.  SimpleProtocolDecoderOut.write() could
>> catch the exception but I think it cannot fire exceptionCaught event
>> correctly because it cannot access AbstractIoFilterChain without breaking
>> the OO design.  Moreover, we cannot guarentee that
>> IoHandler.messageReceived()
>> is executed in the same thread with  ProtocolDecoderOut.write() because
>> there can be an extra thread pool between the codec and the handler.
>>
>> Is it a problem for you to change the decoder state in the decoder itself?
> 
> I'd rather keep my decoder/encoder oblivious of state. I'd like to keep
> them as simple as possible.
> 
> Here's what I propose: make the ProtocolDecoderOut used by
> ProtocolCodecFilter pluggable and make the default behave exactly as
> today. Add the method flush() to ProtocolDecoderOut. Change
> ProtocolCodecFilter.messageReceived() slightly:
> 
>   ProtocolDecoderOutput decoderOut = getDecoderOut(nextFilter,session);
> 
>   try
>   {
>     decoder.decode( session, in, decoderOut );
>   }
>   catch( Throwable t )
>   {
>     ... // No change here
>   }
>   finally
>   {
>     // Dispose the decoder if this session is connectionless.
>     ...
> 
>     // Release the read buffer.
>     in.release();
> 
>     decoderOut.flush();
>   }
> 
> The code
>     Queue queue = decoderOut.getMessageQueue();
>     while( !queue.isEmpty() )
>     {
>       nextFilter.messageReceived( session, queue.pop() );
>     }
> would move into the flush() method of SimpleProtocolDecoderOutput.
> 
> Now I could extend ProtocolCodecFilter and override getDecoderOut() to
> use my non-queuing PDO instead. It would look something like:
> 
> public class NonQueingProtocolDecoderOut implements ProtocolDecoderOutput
> {
>   private final IoSession session;
>   private final NextFilter nextFilter;
>   public NonQueingProtocolDecoderOut(IoSession session, NextFilter
> nextFilter)
>   {
>     this.session = session;
>     this.netxFilter = nextFilter;
>   }
> 
>   public void write( Object message )
>   {
>     nextFilter.messageReceived( session, message );
>   }
> 
>   public void flush() {
>   }
> }
> 
> This solution avoids calling IoHandler.messageReceived() directly. It
> uses the NextFilter which will handle the exceptionCaught() as expected
> so the exception will never be thrown into the PDO.
> 
> The threading issue is of course still a problem but that is something
> we would need to document. It doesn't make any sense to add a
> ThredPoolFilter after the ProtocolCodecFilter if you want to achieve
> what I want to.
> 
> I guess the flush() method should only be called by MINA so adding that
> to the interface could be a problem. But it could also be a feature? I
> could actually achieve what I want by calling flush from my decoder with
> the SimpleProtocolDecoderOutput!
> 
> WDYT?
> 

If no one objects I will go ahead and make these changes.

One concern I have is with the NextFilter. Right now the PDO is being
cached in the session. Using the approach I described above it will hold
on to the NextFilter instance. Is that safe? Is there any reason why we
can't create a new PDO each time we need one instead of caching one in
the session?

-- 
Niklas Therning
Software Architect
www.spamdrain.net

Mime
View raw message