ProtocolParser redux

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

ProtocolParser redux

Erik Svensson-2
Howdy folks,

Now I finally have a working ProtocolParser and ParserProtocolFilter.
To get it working was a journey of discovery and I found a strange thing or
two.

I wrote a a ProtocolParser, a ParserProtocolFilter and set it up like this:

   controller.setProtocolChainInstanceHandler(new
DefaultProtocolChainInstanceHandler() {
        public ProtocolChain poll() {
          ProtocolChain protocol_chain = protocolChains.poll();
          if (protocol_chain == null) {
            protocol_chain = new DefaultProtocolChain();
            protocol_chain.addFilter(parse_filter); // the parser protocol
filter
            protocol_chain.addFilter(xflow_handler); // the module that will
do something intelligent with the created message
            offer(protocol_chain);
          }
          return protocol_chain;
        }
      }); // end overridden method
 
I created a client (with grizzly) that sent 10 messages and the quit.
I fired everything up and found that my server would only read one message
and no more. I fiddled around and finally downloaded the grizzly source and
started the debugger. The incoming byte buffer contained all 10 messages.
The protoocl parser extracted the first message correctly and reported
more_bytes_to_parse as it should to report that there were more messages
that needed handling. To no avail. After a lot of tracing (the fun things
you do on a Saturday evening in front of the telly with the kids singing
along to the eurovision song contest) I found the following snippet:

        if (continousExecution
            && currentPosition == protocolFilters.size() -1
            && (Boolean)ctx.removeAttribute(ProtocolFilter.SUCCESSFUL_READ)
                == Boolean.TRUE) {
            reinvokeChain = true;
        }

where 'continousExecution' looked interesting. Looking through the javadocs
I found a method for setting it in DefaultProtocolChain. Changing my
code above to the not-so-elegant:

   controller.setProtocolChainInstanceHandler(new
DefaultProtocolChainInstanceHandler() {
        public ProtocolChain poll() {
          ProtocolChain protocol_chain = protocolChains.poll();
          if (protocol_chain == null) {
            protocol_chain = new DefaultProtocolChain();
            protocol_chain.addFilter(parse_filter);
            protocol_chain.addFilter(xflow_handler);
        ((DefaultProtocolChain)protocol_chain).setContinuousExecution(true);
// ugly
           
            offer(protocol_chain);
                   
          }
          return protocol_chain;
        }
      }); // end overridden method

Now things works as it should!

Now I'm wondering about this. Why isn't the procotol chain re-invoked by
default if the byte buffer isn't empty? If we read again from the socket,
the remaining bytes from before are still there (unless something/someone
clears the buffer which means we lose messages) which means that we parse a
message that we recievced in the previous read. This inevitably means that
we will be behind on handling messages or maybe not handle some messages at
all.
I therefor propose that the default behaviour is as if continousExecution is
true. PostExecute() could be rewritten to provide state information on
whether or not the chain should be re-invoked and act on that.
If that is not palatable, how about adding the setContinousExecution()
method to the ProtocolChain interface?

cheers
/Erik Svensson, SIX AB


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: ProtocolParser redux

Jeanfrancois Arcand-2
Hi Erik,

Erik Svensson wrote:
> Howdy folks,
>
> Now I finally have a working ProtocolParser and ParserProtocolFilter.
> To get it working was a journey of discovery and I found a strange thing or
> two.

:-) We need to improve the journey of others, so feel free to ask for
changes :-)

>
> I wrote a a ProtocolParser, a ParserProtocolFilter and set it up like this:
>
>    controller.setProtocolChainInstanceHandler(new
> DefaultProtocolChainInstanceHandler() {
>         public ProtocolChain poll() {
>           ProtocolChain protocol_chain = protocolChains.poll();
>           if (protocol_chain == null) {
>             protocol_chain = new DefaultProtocolChain();
>             protocol_chain.addFilter(parse_filter); // the parser protocol
> filter
>             protocol_chain.addFilter(xflow_handler); // the module that will
> do something intelligent with the created message
>             offer(protocol_chain);
>           }
>           return protocol_chain;
>         }
>       }); // end overridden method
>  
> I created a client (with grizzly) that sent 10 messages and the quit.
> I fired everything up and found that my server would only read one message
> and no more. I fiddled around and finally downloaded the grizzly source and
> started the debugger. The incoming byte buffer contained all 10 messages.
> The protoocl parser extracted the first message correctly and reported
> more_bytes_to_parse as it should to report that there were more messages
> that needed handling. To no avail. After a lot of tracing (the fun things
> you do on a Saturday evening in front of the telly with the kids singing
> along to the eurovision song contest)

LOL!

I found the following snippet:

>
>         if (continousExecution
>             && currentPosition == protocolFilters.size() -1
>             && (Boolean)ctx.removeAttribute(ProtocolFilter.SUCCESSFUL_READ)
>                 == Boolean.TRUE) {
>             reinvokeChain = true;
>         }
>
> where 'continousExecution' looked interesting. Looking through the javadocs
> I found a method for setting it in DefaultProtocolChain. Changing my
> code above to the not-so-elegant:
>
>    controller.setProtocolChainInstanceHandler(new
> DefaultProtocolChainInstanceHandler() {
>         public ProtocolChain poll() {
>           ProtocolChain protocol_chain = protocolChains.poll();
>           if (protocol_chain == null) {
>             protocol_chain = new DefaultProtocolChain();
>             protocol_chain.addFilter(parse_filter);
>             protocol_chain.addFilter(xflow_handler);
>         ((DefaultProtocolChain)protocol_chain).setContinuousExecution(true);
> // ugly

Agree.

>            
>             offer(protocol_chain);
>                    
>           }
>           return protocol_chain;
>         }
>       }); // end overridden method
>
> Now things works as it should!
>
> Now I'm wondering about this.

As usual, that's my fault. Well, not only :-) I've added the method
setContinuousExecution() to reduce the numbers of thread switch when the
byte buffer contains more than one "message" and your parser has just
parsed the first one. This improve performance of the http parser and
because you always do: read -> parse -> read again. The last operation
is also important because for http, once you have send the response, the
browser might close the connection, so you catch the connection close
when executing the 'read again' operation.


  Why isn't the procotol chain re-invoked by
> default if the byte buffer isn't empty? If we read again from the socket,
> the remaining bytes from before are still there (unless something/someone
> clears the buffer which means we lose messages) which means that we parse a
> message that we recievced in the previous read. This inevitably means that
> we will be behind on handling messages or maybe not handle some messages at
> all.
> I therefor propose that the default behaviour is as if continousExecution is
> true. PostExecute() could be rewritten to provide state information on
> whether or not the chain should be re-invoked and act on that.

Let me take a look :-)

> If that is not palatable, how about adding the setContinousExecution()
> method to the ProtocolChain interface?

Let me make sure I understand why the current behavior doesn't work, as
the setContinuousExecution() method should not have any effect on the
ProtocolParser. The Parser should be invoked until there is no more
bytes available. Can you file an issue here:

https://grizzly.dev.java.net/issues/

So we can follow up.

Thanks!!

-- Jeanfrancois


>
> cheers
> /Erik Svensson, SIX AB
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

sdo
Reply | Threaded
Open this post in threaded view
|

Re: ProtocolParser redux

sdo
In reply to this post by Erik Svensson-2
+1 on making setContinuousExecution the default for protocol parsers;
I'd meant to mention that explicitly in my previous message.

-Scott

On Mon, 2008-02-11 at 08:18, Erik Svensson wrote:

> Howdy folks,
>
> Now I finally have a working ProtocolParser and ParserProtocolFilter.
> To get it working was a journey of discovery and I found a strange thing or
> two.
>
> I wrote a a ProtocolParser, a ParserProtocolFilter and set it up like this:
>
>    controller.setProtocolChainInstanceHandler(new
> DefaultProtocolChainInstanceHandler() {
>         public ProtocolChain poll() {
>           ProtocolChain protocol_chain = protocolChains.poll();
>           if (protocol_chain == null) {
>             protocol_chain = new DefaultProtocolChain();
>             protocol_chain.addFilter(parse_filter); // the parser protocol
> filter
>             protocol_chain.addFilter(xflow_handler); // the module that will
> do something intelligent with the created message
>             offer(protocol_chain);
>           }
>           return protocol_chain;
>         }
>       }); // end overridden method
>  
> I created a client (with grizzly) that sent 10 messages and the quit.
> I fired everything up and found that my server would only read one message
> and no more. I fiddled around and finally downloaded the grizzly source and
> started the debugger. The incoming byte buffer contained all 10 messages.
> The protoocl parser extracted the first message correctly and reported
> more_bytes_to_parse as it should to report that there were more messages
> that needed handling. To no avail. After a lot of tracing (the fun things
> you do on a Saturday evening in front of the telly with the kids singing
> along to the eurovision song contest) I found the following snippet:
>
>         if (continousExecution
>             && currentPosition == protocolFilters.size() -1
>             && (Boolean)ctx.removeAttribute(ProtocolFilter.SUCCESSFUL_READ)
>                 == Boolean.TRUE) {
>             reinvokeChain = true;
>         }
>
> where 'continousExecution' looked interesting. Looking through the javadocs
> I found a method for setting it in DefaultProtocolChain. Changing my
> code above to the not-so-elegant:
>
>    controller.setProtocolChainInstanceHandler(new
> DefaultProtocolChainInstanceHandler() {
>         public ProtocolChain poll() {
>           ProtocolChain protocol_chain = protocolChains.poll();
>           if (protocol_chain == null) {
>             protocol_chain = new DefaultProtocolChain();
>             protocol_chain.addFilter(parse_filter);
>             protocol_chain.addFilter(xflow_handler);
>         ((DefaultProtocolChain)protocol_chain).setContinuousExecution(true);
> // ugly
>            
>             offer(protocol_chain);
>                    
>           }
>           return protocol_chain;
>         }
>       }); // end overridden method
>
> Now things works as it should!
>
> Now I'm wondering about this. Why isn't the procotol chain re-invoked by
> default if the byte buffer isn't empty? If we read again from the socket,
> the remaining bytes from before are still there (unless something/someone
> clears the buffer which means we lose messages) which means that we parse a
> message that we recievced in the previous read. This inevitably means that
> we will be behind on handling messages or maybe not handle some messages at
> all.
> I therefor propose that the default behaviour is as if continousExecution is
> true. PostExecute() could be rewritten to provide state information on
> whether or not the chain should be re-invoked and act on that.
> If that is not palatable, how about adding the setContinousExecution()
> method to the ProtocolChain interface?
>
> cheers
> /Erik Svensson, SIX AB
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]