Hello,
Thanks for your comments.
How about second version:
# HG changeset patch
# User ykirpichev@gmail.com
# Date 1370964275 -14400
# Branch nopush_fix_2
# Node ID 14de55787a48327019d549d48abf2631e294b4d8
# Parent 725fb71ab1a60bd48b0afb8b001b5349f5054cb1
SPDY: fix nopush cleanup
diff -r 725fb71ab1a6 -r 14de55787a48 src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http_spdy.c Tue Jun 11 19:24:35 2013 +0400
@@ -504,6 +504,51 @@
clcf = ngx_http_get_module_loc_conf(sc->http_connection->conf_ctx,
ngx_http_core_module);
+ // all data is sent, can clean nopush if necessary
+ if (wev->ready)
+ {
+ int tcp_nodelay;
+
+ if (c->tcp_nopush == NGX_TCP_NOPUSH_SET) {
+ if (ngx_tcp_push(c->fd) == -1) {
+ ngx_connection_error(c, ngx_socket_errno, ngx_tcp_push_n "
failed");
+ return NGX_ERROR;
+ }
+
+ c->tcp_nopush = NGX_TCP_NOPUSH_UNSET;
+ tcp_nodelay = ngx_tcp_nodelay_and_tcp_nopush ? 1 : 0;
+
+ } else {
+ tcp_nodelay = 1;
+ }
+
+ if (tcp_nodelay
+ && clcf->tcp_nodelay
+ && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET)
+ {
+ ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "tcp_nodelay");
+
+ if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
+ (const void *) &tcp_nodelay, sizeof(int))
+ == -1)
+ {
+#if (NGX_SOLARIS)
+ /* Solaris returns EINVAL if a socket has been shut down */
+ c->log_error = NGX_ERROR_IGNORE_EINVAL;
+#endif
+
+ ngx_connection_error(c, ngx_socket_errno,
+ "setsockopt(TCP_NODELAY) failed");
+
+ c->log_error = NGX_ERROR_INFO;
+ return NGX_ERROR;
+ }
+
+ c->tcp_nodelay = NGX_TCP_NODELAY_SET;
+ }
+
+ }
+
if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
return NGX_ERROR; /* FIXME */
}
2013/6/11 Maxim Dounin <mdounin@mdounin.ru>
> Hello!
>
> On Tue, Jun 11, 2013 at 01:00:42PM +0400, Yury Kirpichev wrote:
>
> > Could you please take a look at patch below.
> > I've tried to fix problem that TCP_CORK option is not cleaned in SPDY
> case.
>
> The patch looks wrong.
>
> - It introduces layering violation and build failure --without-http
> as a result. You may have better luck focusing on a problem you
> want to fix, and avoiding unrelated changes as much as possible.
>
> - It tries to restore nopush after each c->send_chain() call,
> which looks suboptimal. It probably should be done if there are no
> pending data to send.
>
> --
> Maxim Dounin
> http://nginx.org/en/donation.html
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel@nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
