Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channels being possibly reused in with-channel, causing exceptions with threads #76

Closed
cndreisbach opened this issue Aug 9, 2013 · 4 comments

Comments

@cndreisbach
Copy link

Under heavy load, I am seeing what I believe to be channels being re-used. I am getting an IllegalStateException from this line of http-kit code when I try to set a close handler on what I think should be a new channel.

Here are the details:

I am using http-kit with a REST API I am building to return large amounts of data in JSON, CSV, and XML. Specifically, I am using it so that I can stream these responses, as they may be many megabytes and can take time to assemble on the server.

You can see the code I am using here: https://github.com/cndreisbach/qu/blob/streaming/src/cfpb/qu/views.clj#L368

In this code, I am:

  1. Creating a PipedInputStream and PipedOutputStream and connecting them.
  2. In a separate thread, calling generate-stream from Cheshire to generate a stream of JSON and send it into the PipedOutputStream.
  3. In the original thread, creating a channel from the request.
  4. I read from the PipedInputStream. As long as I can keep reading from this stream, I send chunks over my channel. As soon as I can't, I close the channel.

Running with normal load, this works great! As soon as I put JMeter on it for some load testing, it goes bananas. I am getting this IllegalStateException from trying to set the close handler. Even if I stop setting the close handler, I am getting a different error (org.apache.http.MalformedChunkCodingException: Bad chunk header from JMeter, not from http-kit, but I think this is related.)

Has anybody seen this type of behavior before? Am I crazy for using multiple threads in my response and using PipedInput/OutputReader for managing streaming data?

@shenfeng
Copy link
Member

shenfeng commented Aug 9, 2013

http-kit reuses the AsyncChannel object. AsyncChannel is per connection. A connection is a TCP connection. Since browser or JMeter will try to use the TCP connection, AsyncChannel get reused. When reused, AsyncChannel get reseted https://github.com/http-kit/http-kit/blob/master/src/java/org/httpkit/server/AsyncChannel.java#L73

Since the reset is lazyset, maybe this is where the problem is.

http-kit only allow set the closeHandler once, maybe this is a bad design. How do you think about it?

As regarding the multiple threads, how about this:
Do not use PipedInputStream or PipedOutputStream. Instead of write to the PipedOutputStream, just write to channel. http-kit is multi-threaded, blocking a few thread will be fine. Set :thread option to a larger value will be fine http://http-kit.org/server.html#options

@cndreisbach
Copy link
Author

This helped so much! Thank you, @shenfeng.

As for only allowing the closeHandler once, I think it's ok, although it is confusing since the reset is lazyset and you can encounter a situation where you think the closeHandler is not set but it actually is.

When you say "just write to channel," what do you mean, exactly? I am generating a stream of JSON or XML text and need an OutputStream for it to go into. The channel's not an OutputStream, so I'm not certain the best way to go about that.

@shenfeng
Copy link
Member

shenfeng commented Aug 9, 2013

I will try to reproduce and fix the issue with closeHandler, It maybe a multi thread bug of http-kit.

The channel's not an OutputStream

You can create a method, like:
ch->outputstream

(defn ch->outputstream [ch]
  (proxy [java.io.OutputStream] []
    (close []
       )
    (write
      ([bs] ;; bytes of byte
             ;; write to channel
         )
      ([^bytes bs off  len]
         ))))

@shenfeng
Copy link
Member

Hi, I look at the code carefully, the reset is threadsafe, should ok.
Can you help by show test case to reproduce the problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants