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

Already on GitHub? Sign in to your account

add partial rack hijack for WEBrick #533

Merged
merged 2 commits into from Mar 29, 2013

Conversation

Projects
None yet
4 participants
Contributor

dahakawang commented Mar 27, 2013

No description provided.

@rkh rkh commented on an outdated diff Mar 27, 2013

lib/rack/handler/webrick.rb
@@ -69,9 +76,19 @@ def service(req, res)
res[k] = vs.split("\n").join(", ")
end
}
- body.each { |part|
- res.body << part
- }
+
+ io_lambda = headers["rack.hijack"]
+ if io_lambda
+ rd, wr = IO.pipe
+ res.body = rd
+ res.chunked = true
+ res.body
@rkh

rkh Mar 27, 2013

Member

What's this line for?

@rkh rkh commented on an outdated diff Mar 27, 2013

lib/rack/handler/webrick.rb
@@ -46,7 +47,11 @@ def service(req, res)
"rack.multiprocess" => false,
"rack.run_once" => false,
- "rack.url_scheme" => ["yes", "on", "1"].include?(ENV["HTTPS"]) ? "https" : "http"
+ "rack.url_scheme" => ["yes", "on", "1"].include?(ENV["HTTPS"]) ? "https" : "http",
+
+ "rack.hijack?" => true,
+ "rack.hijack" => lambda { raise RuntimeError, "only partial hijack is supported."},
@rkh

rkh Mar 27, 2013

Member

Should this be a NotImplementedError maybe?

Member

rkh commented Mar 28, 2013

OK, this looks pretty sweet.

@raggi wdyt?

Also, we might wanna add tests.

Contributor

dahakawang commented Mar 29, 2013

@rkh im going to add some test cases later.. 😃

Member

rkh commented Mar 29, 2013

Good work! WEBrick is threaded internally for the part that's streaming out the body, is it? Can this handle more than one connection at a time or will one hijacked dangling connection block everything?

Contributor

dahakawang commented Mar 29, 2013

@rkh im not familiar with WEBrick‘s internal implementation, but iv done some experiments which show multiple connections can be established simultaneously with a hijacked controller(within rails).

@rkh rkh added a commit that referenced this pull request Mar 29, 2013

@rkh rkh Merge pull request #533 from dahakawang/master
add partial rack hijack for WEBrick
232ed1e

@rkh rkh merged commit 232ed1e into rack:master Mar 29, 2013

1 check passed

default The Travis build passed
Details

@spastorino @tenderlove this is making Rails static file serving in development very slow (see rails/rails#18828) - this doesn't seem right to me, wdyt?

Member

tenderlove replied Mar 22, 2015

TBH I don't understand why we're sending static assets through the whole Rack pipeline in development mode. If we know that static assets are always going to sit in a certain location, why don't we add a WEBRick File handler to point at that directory? Not only could it serve up the files faster, but it wouldn't go through Rack::Lock in dev mode (which means static assets could be served in parallel).

@tenderlove because the asset files are coming from Sprockets not from files in /public/assets typically (unless you've run rake assets:precompile but that's a whole other set of issues).

Even if we did somehow manage to make assets work via a WEBRick file handler, do we want to send rendered output five bytes at a time? 😄

Member

tenderlove replied Mar 23, 2015

because the asset files are coming from Sprockets not from files in /public/assets typically

So they're not actually static? If they are actually static, I don't think there's anything preventing us from pointing a file handler in the app/assets directory.

Even if we did somehow manage to make assets work via a WEBRick file handler, do we want to send rendered output five bytes at a time?

We can separate the byte sizes between static and dynamic requests.

So they're not actually static? If they are actually static, I don't think there's anything preventing us from pointing a file handler in the app/assets directory

The app/assets directory structure isn't the url structure - for example app/assets/images/logo.png => /assets/logo.png.

We can separate the byte sizes between static and dynamic requests

But surely five bytes is too small even for dynamic requests?

Member

tenderlove replied Mar 23, 2015

Ya, I think we can increase the byte size. I just really want to get asset serving out of Rack::Lock. We have many files, and serving them serially us super painful.

The app/assets directory structure isn't the url structure - for example app/assets/images/logo.png => /assets/logo.png.

Do you know if that's the only transformation? We can mount a webrick file handler at any URL and have the base be somewhere else. I just don't know the rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment