Right, metrics != null has nothing to do with this, ideally it should be
outside of that block. I will fix it.

Thanks,
Ruwan

On Sun, May 16, 2010 at 1:15 PM, Andreas Veithen
<[email protected]>wrote:

> What about the metrics != null part of the condition?
>
> Andreas
>
> On Sun, May 16, 2010 at 03:55, Ruwan Linton <[email protected]>
> wrote:
> > Hi Andreas,
> >
> > Although the existing code calls requestCompletionTime unnecessarily even
> > before the encoder gets completed, it captures the requestCompletionTime
> > even in error cases. For example the request transmission not getting
> > completed properly. Basically when ever we write some bytes to the
> channel,
> > we assume the request is getting completed and keep on increasing that
> > value. I agree there is a perf hit though.
> >
> > The whole point of the CCD, is to capture connection debug information to
> > the best possible level so that analyzing and issue on a message
> > transmission is straightforward.
> >
> > One thing we could do is to have the requestCompletionTime call within
> the
> > encoder.isCompleted as well as in the catch block. I guess we need a
> generic
> > Exception catch block rather just IOException catch block in that case.
> >
> > Thanks,
> > Ruwan
> >
> > On Sun, May 16, 2010 at 12:54 AM, Andreas Veithen
> > <[email protected]> wrote:
> >>
> >> Could the people who did the ClientConnectionDebug stuff please have a
> >> look at the TODO item below. I don't think that the code is executed
> >> in the right place.
> >>
> >> Andreas
> >>
> >>
> >> ---------- Forwarded message ----------
> >> From:  <[email protected]>
> >> Date: Sat, May 15, 2010 at 21:15
> >> Subject: svn commit: r944701 -
> >>
> >>
> /synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java
> >> To: [email protected]
> >>
> >>
> >> Author: veithen
> >> Date: Sat May 15 19:15:43 2010
> >> New Revision: 944701
> >>
> >> URL: http://svn.apache.org/viewvc?rev=944701&view=rev
> >> Log:
> >> Take into account that the encoder may switch to completed even if
> >> bytesWritten == 0.
> >>
> >> Modified:
> >>
> >>
>  
> synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java
> >>
> >> Modified:
> >>
> synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java
> >> URL:
> >>
> http://svn.apache.org/viewvc/synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java?rev=944701&r1=944700&r2=944701&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> >>
> synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java
> >> (original)
> >> +++
> >>
> synapse/trunk/java/modules/transports/core/nhttp/src/main/java/org/apache/synapse/transport/nhttp/ClientHandler.java
> >> Sat May 15 19:15:43 2010
> >> @@ -555,11 +555,21 @@ public class ClientHandler implements NH
> >>
> >>         try {
> >>             int bytesWritten = outBuf.produceContent(encoder);
> >> -            if (metrics != null && bytesWritten > 0) {
> >> -                if (metrics.getLevel() == MetricsCollector.LEVEL_FULL)
> {
> >> -
> >> metrics.incrementBytesSent(getMessageContext(conn), bytesWritten);
> >> -                } else {
> >> -                    metrics.incrementBytesSent(bytesWritten);
> >> +            if (metrics != null) {
> >> +                if (bytesWritten > 0) {
> >> +                    if (metrics.getLevel() ==
> >> MetricsCollector.LEVEL_FULL) {
> >> +
> >> metrics.incrementBytesSent(getMessageContext(conn), bytesWritten);
> >> +                    } else {
> >> +                        metrics.incrementBytesSent(bytesWritten);
> >> +                    }
> >> +
> >> +                    // TODO: executing this when metrics != 0 &&
> >> bytesWritten > 0 seems strange;
> >> +                    //       shouldn't the condition be
> >> encoder.isCompleted() ?!?!?
> >> +                    ClientConnectionDebug ccd = (ClientConnectionDebug)
> >> +
> >>  context.getAttribute(CLIENT_CONNECTION_DEBUG);
> >> +                    if (ccd != null) {
> >> +                        ccd.recordRequestCompletionTime();
> >> +                    }
> >>                 }
> >>
> >>                 if (encoder.isCompleted()) {
> >> @@ -569,12 +579,6 @@ public class ClientHandler implements NH
> >>                         metrics.incrementMessagesSent();
> >>                     }
> >>                 }
> >> -
> >> -                ClientConnectionDebug ccd = (ClientConnectionDebug)
> >> -                        context.getAttribute(CLIENT_CONNECTION_DEBUG);
> >> -                if (ccd != null) {
> >> -                    ccd.recordRequestCompletionTime();
> >> -                }
> >>             }
> >>
> >>         } catch (IOException e) {
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [email protected]
> >> For additional commands, e-mail: [email protected]
> >>
> >
> >
> >
> > --
> > Ruwan Linton
> > Software Architect & Product Manager, WSO2 ESB; http://wso2.org/esb
> > WSO2 Inc.; http://wso2.org
> >
> > Lean . Enterprise . Middleware
> >
> > phone: +1 408 754 7388 ext 51789
> > email: [email protected]; cell: +94 77 341 3097
> > blog: http://blog.ruwan.org
> > linkedin: http://www.linkedin.com/in/ruwanlinton
> > google: http://www.google.com/profiles/ruwan.linton
> > tweet: http://twitter.com/ruwanlinton
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>


-- 
Ruwan Linton
Software Architect & Product Manager, WSO2 ESB; http://wso2.org/esb
WSO2 Inc.; http://wso2.org

Lean . Enterprise . Middleware

phone: +1 408 754 7388 ext 51789
email: [email protected]; cell: +94 77 341 3097
blog: http://blog.ruwan.org
linkedin: http://www.linkedin.com/in/ruwanlinton
google: http://www.google.com/profiles/ruwan.linton
tweet: http://twitter.com/ruwanlinton

Reply via email to