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.

Sign up for GitHub

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

Jump to bottom

improve timeout_idle documentation wrt H2 #2927

Closed
Francois-v3 opened this issue Mar 4, 2019 · 8 comments
Closed

improve timeout_idle documentation wrt H2 #2927

Francois-v3 opened this issue Mar 4, 2019 · 8 comments
Assignees
@dridi @nigoroll

Comments

@Francois-v3
Copy link

In varnishadm 6.1, documentation of timeout_idle says:

varnishadm param.show timeout_idle timeout_idle

    Value is: 10.000 [seconds]
    Default is: 5.000
    Minimum is: 0.000

    Idle timeout for client connections.

    A connection is considered idle until we have received the full
    request headers.

    This parameter is particularly relevant for HTTP1 keepalive
    connections which are closed unless the next request is
    received before this timeout is reached.

It appears that it's "particularly relevant" for HTTP/2 too ! I would says even more, as there's no more connection header in HTTP/2 ...

# varnishd -V
varnishd (varnish-6.1.1 revision efc2f6c1536cf2272e471f5cff5f145239b19460)
Copyright (c) 2006 Verdens Gang AS
Copyright (c) 2006-2015 Varnish Software AS
@mbgrydeland mbgrydeland self-assigned this Mar 4, 2019
@nigoroll nigoroll changed the title timeout_idle documentation improve timeout_idle documentation wrt H2 Apr 23, 2019
@nigoroll
Copy link
Member

FTR the meaning of timeout_idle is not particularly well defined at the moment. work in this broad area is currently happening in #2980

@daghf
Copy link
Member

daghf commented Sep 25, 2019

With #2980 done, we now take timeout_idle into account also for h/2.

I'll close this ticket now.

@daghf daghf closed this as completed Sep 25, 2019
@daghf
Copy link
Member

daghf commented Sep 25, 2019

Sorry - I noticed #2980 did not update the doc. I'll reopen for now

@daghf daghf reopened this Sep 25, 2019
@dridi
Copy link
Member

dridi commented Sep 25, 2019

Are you sure? I don't remember the specifics but I worked on different timeouts: idle_send_timeout and send_timeout, not timeout_idle. I think we are back to @nigoroll's comment.

@dridi
Copy link
Member

dridi commented Dec 9, 2019

I'm not sure how @nigoroll feels about it but I think we shouldn't discuss this question today. As requested in #2983 (comment) we worked on bringing consistency to timeouts, and while we did not specifically address h2 we have a VIP draft that we may submit to the wiki to be discussed next bugwash.

A side effect of this draft is that it should help us define what timeout_idle would mean for streams-based transports like h2.

@nigoroll
Copy link
Member

nigoroll commented Dec 9, 2019

I agree with @dridi
We have already spent some relevant amount of time on a comprehensive proposal and I would think bugwash time was invested more wisely if spent on that proposal (later).

@nigoroll nigoroll assigned dridi and nigoroll and unassigned mbgrydeland Jan 15, 2020
@nigoroll
Copy link
Member

@dridi and myself still need to finish the timeout-proposal we have already spent quite some time on, but which we did not finish yet. This ticket seems to serve no real purpose, other than reminding me once again that we need to.

@bsdphk
Copy link
Contributor

bsdphk commented May 5, 2021

Timed out.

@bsdphk bsdphk closed this as completed May 5, 2021
dridi added a commit to dridi/varnish-cache that referenced this issue Jun 17, 2022
@dridi
param: Rename idle_send_timeout to resp_idle_interrupt
8dac74a 
Exceptionally, one existing VTC was migrated to the new name immediately
since it relied on debug logs containing 'idle_send_timeout'.

Turning the timeout to an interrupt, I tried to update the parameter
description to better convey what it does. This does not change the
current behavior, but I tried to highlight what is wrong with this
timeout and the mismatch between its purpose for HTTP/1 and h2.

Depending on where we look at the documentation, between the parameter
or VCL variables, we get a slightly different description. None of them
are accurate, but the consensus seems to have always been the parameter
description from before h2 or the 'sess.idle_send_timeout' variable.

For session-level timeouts, we may consider an h2_send_timeout parameter
in the future to distinguish between socket writes and underlying h2
streams timing out on a condvar. This h2_send_timeout parameter would
map to SO_SNDTIMEO for h2 client connections.

Refs varnishcache#2927
dridi added a commit to dridi/varnish-cache that referenced this issue Jun 27, 2022
@dridi
param: Rename idle_send_timeout to resp_idle_interrupt
b19d2c8 
Exceptionally, one existing VTC was migrated to the new name immediately
since it relied on debug logs containing 'idle_send_timeout'.

Turning the timeout to an interrupt, I tried to update the parameter
description to better convey what it does. This does not change the
current behavior, but I tried to highlight what is wrong with this
timeout and the mismatch between its purpose for HTTP/1 and h2.

Depending on where we look at the documentation, between the parameter
or VCL variables, we get a slightly different description. None of them
are accurate, but the consensus seems to have always been the parameter
description from before h2 or the 'sess.idle_send_timeout' variable.

For session-level timeouts, we may consider an h2_send_timeout parameter
in the future to distinguish between socket writes and underlying h2
streams timing out on a condvar. This h2_send_timeout parameter would
map to SO_SNDTIMEO for h2 client connections.

Refs varnishcache#2927
dridi added a commit to dridi/varnish-cache that referenced this issue Jun 27, 2022
@dridi
param: Rename idle_send_timeout to resp_idle_interrupt
0c97b92 
Exceptionally, one existing VTC was migrated to the new name immediately
since it relied on debug logs containing 'idle_send_timeout'.

Turning the timeout to an interrupt, I tried to update the parameter
description to better convey what it does. This does not change the
current behavior, but I tried to highlight what is wrong with this
timeout and the mismatch between its purpose for HTTP/1 and h2.

Depending on where we look at the documentation, between the parameter
or VCL variables, we get a slightly different description. None of them
are accurate, but the consensus seems to have always been the parameter
description from before h2 or the 'sess.idle_send_timeout' variable.

For session-level timeouts, we may consider an h2_send_timeout parameter
in the future to distinguish between socket writes and underlying h2
streams timing out on a condvar. This h2_send_timeout parameter would
map to SO_SNDTIMEO for h2 client connections.

Refs varnishcache#2927
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dridi dridi

@nigoroll nigoroll

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@bsdphk @mbgrydeland @dridi @nigoroll @daghf @Francois-v3