[jetty-dev] SPDY window size update, a bug?

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

[jetty-dev] SPDY window size update, a bug?

Will
Hello,

I have been playing around with Jetty's shiny new SPDY support and ran
into a problem where my SPDY sessions were timing out, apparently
because a PING response wasn't being sent to the client (Chrome 17).

I tracked this down to StandardSession, where a flush call was being
skipped with two items in the queue, a DataFrame and the PingFrame
response. The flush was skipped because DataFrameBytes.getByteBuffer()
was returning null, which in turn was caused by the Stream window size
being <= 0.

When the complete() method of DataFrameBytes is called it calls
Stream.updateWindowSize(-length):

https://github.com/eclipse/jetty.project/blob/jetty-8/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java#L1037

Is that correct? I don't follow the logic for updating the window size
that way, but I am very new to both SPDY and the Jetty code base.
Removing the window size update does fix the client PING timeouts.

Thanks!
Will
_______________________________________________
jetty-dev mailing list
[hidden email]
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] SPDY window size update, a bug?

Simone Bordet-2
Hi,

On Wed, Mar 28, 2012 at 07:40, Will <[hidden email]> wrote:

> Hello,
>
> I have been playing around with Jetty's shiny new SPDY support and ran
> into a problem where my SPDY sessions were timing out, apparently
> because a PING response wasn't being sent to the client (Chrome 17).
>
> I tracked this down to StandardSession, where a flush call was being
> skipped with two items in the queue, a DataFrame and the PingFrame
> response. The flush was skipped because DataFrameBytes.getByteBuffer()
> was returning null, which in turn was caused by the Stream window size
> being <= 0.
>
> When the complete() method of DataFrameBytes is called it calls
> Stream.updateWindowSize(-length):
>
> https://github.com/eclipse/jetty.project/blob/jetty-8/jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/StandardSession.java#L1037
>
> Is that correct? I don't follow the logic for updating the window size
> that way, but I am very new to both SPDY and the Jetty code base.
> Removing the window size update does fix the client PING timeouts.

I am not sure that flow control is implemented in Chromium/Chrome 17,
so perhaps we need a switch to turn it off on server side too.
I think the update of the window size is right, but we're stalling
PING frames (and other channels too), so that looks like a bug.
Investigating.

Simon
--
http://cometd.org
http://intalio.com
http://bordet.blogspot.com
----
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz
_______________________________________________
jetty-dev mailing list
[hidden email]
https://dev.eclipse.org/mailman/listinfo/jetty-dev
Reply | Threaded
Open this post in threaded view
|

Re: [jetty-dev] SPDY window size update, a bug?

Simone Bordet-2
Hi,

On Wed, Mar 28, 2012 at 09:45, Simone Bordet <[hidden email]> wrote:
> I am not sure that flow control is implemented in Chromium/Chrome 17,
> so perhaps we need a switch to turn it off on server side too.
> I think the update of the window size is right, but we're stalling
> PING frames (and other channels too), so that looks like a bug.
> Investigating.

Confirmed as bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=375509

Simon
--
http://cometd.org
http://intalio.com
http://bordet.blogspot.com
----
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz
_______________________________________________
jetty-dev mailing list
[hidden email]
https://dev.eclipse.org/mailman/listinfo/jetty-dev