Commit 5f654609 authored by Tatsuhiro Tsujikawa's avatar Tatsuhiro Tsujikawa

nghttpx: Don't change pushed stream's priority

There is a discussion in httpbis mailing list which argues that
dependency tree is for client, and changing it in server side is not
what client expects.
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JulSep/0416.html

Currently, we make pushed stream depend on the parent stream of
associated stream (that is main HTML in most of the cases), so that
associated stream and pushed stream become siblings.  In this case, we
also observed that these resources complete each other to get its
parent weight.  This means that the delivery of associated stream is
delayed by pushed streams.

So at this moment, it is not a good idea to change pushed stream
priority in a way we do currently.
parent 41b2745d
......@@ -1503,13 +1503,6 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) {
return 0;
}
if (downstream->get_assoc_stream_id() != -1) {
rv = adjust_pushed_stream_priority(downstream);
if (rv != 0) {
return -1;
}
}
http2::copy_headers_to_nva_nocopy(nva, resp.fs.headers());
if (!get_config()->http2_proxy) {
......@@ -1605,68 +1598,6 @@ int Http2Upstream::on_downstream_body(Downstream *downstream,
return 0;
}
int Http2Upstream::adjust_pushed_stream_priority(Downstream *downstream) {
int rv;
// We only change pushed stream. The pushed stream has
// assoc_stream_id which is not -1.
auto assoc_stream_id = downstream->get_assoc_stream_id();
auto stream_id = downstream->get_stream_id();
auto assoc_stream = nghttp2_session_find_stream(session_, assoc_stream_id);
auto stream = nghttp2_session_find_stream(session_, stream_id);
// By default, downstream depends on assoc_stream. If its
// relationship is changed, then we don't change priority.
if (!assoc_stream || assoc_stream != nghttp2_stream_get_parent(stream)) {
return 0;
}
// We are going to make stream depend on dep_stream which is the
// parent stream of assoc_stream, if the content-type of stream
// indicates javascript or css.
auto dep_stream = nghttp2_stream_get_parent(assoc_stream);
if (!dep_stream) {
return 0;
}
const auto &resp = downstream->response();
auto ct = resp.fs.header(http2::HD_CONTENT_TYPE);
if (!ct) {
return 0;
}
if (!util::istarts_with_l(ct->value, "application/javascript") &&
!util::istarts_with_l(ct->value, "text/css") &&
// for polymer...
!util::istarts_with_l(ct->value, "text/html")) {
return 0;
}
auto dep_stream_id = nghttp2_stream_get_stream_id(dep_stream);
auto weight = nghttp2_stream_get_weight(assoc_stream);
nghttp2_priority_spec pri_spec;
nghttp2_priority_spec_init(&pri_spec, dep_stream_id, weight, 0);
rv = nghttp2_session_change_stream_priority(session_, stream_id, &pri_spec);
if (nghttp2_is_fatal(rv)) {
ULOG(FATAL, this) << "nghttp2_session_change_stream_priority() failed: "
<< nghttp2_strerror(rv);
return -1;
}
if (rv == 0) {
if (LOG_ENABLED(INFO)) {
ULOG(INFO, this) << "Changed pushed stream priority: pushed stream("
<< stream_id << ") now depends on stream("
<< dep_stream_id << ") with weight " << weight;
}
}
return 0;
}
// WARNING: Never call directly or indirectly nghttp2_session_send or
// nghttp2_session_recv. These calls may delete downstream.
int Http2Upstream::on_downstream_body_complete(Downstream *downstream) {
......
......@@ -118,10 +118,6 @@ public:
DefaultMemchunks *get_response_buf();
// Changes stream priority of |downstream|, which is assumed to be a
// pushed stream.
int adjust_pushed_stream_priority(Downstream *downstream);
private:
DefaultMemchunks wb_;
std::unique_ptr<HttpsUpstream> pre_upstream_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment