Change idle_timeout to max_idle_timeout #3099

ianswett · 2019-10-16T17:55:07Z

This makes idle timeout symmetric by stating that a peer will close the connection at or soon after the idle timeout expires, creating clear expectations of idle timeout behavior. This also renames the transport parameter idle_timeout to max_idle_timeout for clarity.

Fixes #2602
Takes some of the improved text from #2745

This makes idle timeout symmetric by stating that a peer will close the connection at or soon after the idle timeout expires, creating clear expectations of idle timeout behavior. Fixes #2602 Takes some of the improved text from #2745

janaiyengar · 2019-10-16T18:06:12Z

draft-ietf-quic-transport.md

+shorter period advertised by the peer.  By announcing an idle timeout,
+an endpoint commits to initiating an immediate close ({{immediate-close}})
+if it abandons a connection prior to both its peers and its own idle timeout
+expiring.


I don't think I follow this sentence. What is the endpoint committing to? To closing the connection at min(local_timeout, remote_timeout)?

It's committing to explicitly closing(and not silently closing) if it's before min(local_timeout, remote_timeout). This was adapted from MT's PR.

draft-ietf-quic-transport.md

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

martinthomson

This doesn't read particularly well as written. I've suggested something, but I'm not entirely sure that it works.

draft-ietf-quic-transport.md

kazuho

I have suggestions for parts that did not read well to me, but otherwise LGTM.

draft-ietf-quic-transport.md

martinthomson

Thanks for doing this.

I wonder if we have any place where we can state the principle that drives this. That is, "after the time indicated in max_idle_timeout, the endpoint will no longer send nor accept packets."

draft-ietf-quic-transport.md

MikeBishop · 2019-10-30T20:19:38Z

draft-ietf-quic-transport.md

+a packet containing frames other than ACK or PADDING (an ack-eliciting packet;
+see {{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been
+sent since last receiving a packet.  Restarting when sending packets ensures
+that connections do not prematurely time out when initiating new activity.


If you're going to explain why you restart when sending, you might need to explain why you don't restart when sending subsequent packets. Perhaps "when sending one or more packets" might help point this explanation at bursts, rather than at individual packets?

Also pre-existing.

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

janaiyengar

A few comments. I think it would help readability immensely if you could articulate a new timeout period, Idle Timeout (ITO) explicitly.

Also, can you change the title of the PR? It's not particularly clarifying.

draft-ietf-quic-transport.md

janaiyengar · 2019-11-01T21:07:27Z

draft-ietf-quic-transport.md

+Each endpoint advertises a different max_idle_timeout, but the effective value
+is determined by taking the minimum of the two advertised values. By announcing
+a max_idle_timeout, endpoints commit to initiating an immediate close
+({{immediate-close}}) if it abandons a connection prior to the effective value.


I don't understand this sentence... what does it mean for an endpoint to initiate an immediate close if it abandons the connection prior to the min? I thought that the endpoint was to initiate an immediate close at the min. Why are we talking about abandoning?

draft-ietf-quic-transport.md

janaiyengar · 2019-11-01T21:09:42Z

draft-ietf-quic-transport.md

+received and processed successfully.  The timer is also restarted when sending
+a packet containing frames other than ACK or PADDING (an ack-eliciting packet;
+see {{QUIC-RECOVERY}}), but only if no other ack-eliciting packets have been
+sent since last receiving a packet.  Restarting when sending packets ensures


Can you write some rationale for these restarting rules?

Also pre-existing text, so maybe in a different PR?

janaiyengar · 2019-11-01T21:17:18Z

draft-ietf-quic-transport.md

+
+An endpoint that sends packets near the end of the idle timeout period
+risks having those packets discarded if its peer enters the draining state
+before the packets arrive.  If a peer could time out within an Probe Timeout


This is not a great construction: "If a peer could time out within a Probe Timeout". It's also confusing ... maybe change this to "When an endpoint is about to send data that cannot be retried safely, it is possible that the peer may have reached the end of its idle timeout period before this data arrives at the peer. To avoid such situations, it is recommended that an endpoint test for liveness when it expects that the peer might be close, within a Probe Timeout (PTO; see Section ...) period for instance, to the end of its idle timeout period."

Agreed, I'll file an issue to address your and Mike's comments.

I think the text should be fixed here... why introduce text here and then fix it in a new PR?

I'm not introducing this text, it just moved, hence the different PR, which is editorial.

draft-ietf-quic-transport.md

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

ianswett · 2019-11-02T22:49:10Z

@MikeBishop and @janaiyengar I filed a new issue(#3184) for your comments on the existing idle timeout text. I hope all of those comments can be addressed in an editorial PR.

martinthomson · 2019-11-03T23:54:48Z

draft-ietf-quic-transport.md

-processed successfully.  The timer is also restarted when sending an
+Each endpoint advertises a max_idle_timeout, but the effective value
+at an endpoint is computed as the minimum of the two advertised values. By announcing
+a max_idle_timeout, an endpoint commits to initiating an immediate close


a max_idle_timeout, an endpoint declares that after being idle for longer than the advertised time it will not send any packets and it could generate a stateless reset ({{stateless-reset}}) in reaction to any received packet. An endpoint commits to initiating an immediate close [...]

draft-ietf-quic-transport.md

ianswett · 2019-11-20T02:06:59Z

This is failing due to a QPACK error, but I believe the PR is done.

As noted above, I opened #3184 for the many comments about the existing text. I can take a stab at this, but I'd like to do that separately from this design change.

draft-ietf-quic-transport.md

janaiyengar

One more comment

janaiyengar · 2019-12-11T00:05:08Z

draft-ietf-quic-transport.md

+risks having those packets discarded if its peer enters the draining state
+before the packets arrive.  If a peer could time out within a Probe Timeout
+(PTO; see Section 6.2.2 of {{QUIC-RECOVERY}}), it is advisable to test for
+liveness before sending any data that cannot be retried safely.  Note that it


What does it mean to test for liveness? Can you make this more explicit?

@janaiyengar

From @janaiyengar and @MikeBishop comments on #3099 Fixes #3184

Idle timeout indicates you will timeout then

61a3725

This makes idle timeout symmetric by stating that a peer will close the connection at or soon after the idle timeout expires, creating clear expectations of idle timeout behavior. Fixes #2602 Takes some of the improved text from #2745

ianswett added the -transport label Oct 16, 2019

janaiyengar reviewed Oct 16, 2019

View reviewed changes

Update draft-ietf-quic-transport.md

dc5b220

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

larseggert mentioned this pull request Oct 16, 2019

Idle timeout needs more description and a recommendation #2602

Closed

martinthomson requested changes Oct 16, 2019

View reviewed changes

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

ianswett added 3 commits October 28, 2019 20:33

Martin's suggestion and some reordering

2537a1a

Update draft-ietf-quic-transport.md

61dcaad

Update draft-ietf-quic-transport.md

6fd0ac8

kazuho reviewed Oct 29, 2019

View reviewed changes

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

Kazuho's suggestions

e4dd819

nibanks approved these changes Oct 29, 2019

View reviewed changes

martinthomson approved these changes Oct 29, 2019

View reviewed changes

MikeBishop approved these changes Oct 30, 2019

View reviewed changes

ianswett and others added 5 commits November 1, 2019 13:24

Update draft-ietf-quic-transport.md

2234623

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

Update draft-ietf-quic-transport.md

11d8da8

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

Update draft-ietf-quic-transport.md

508413d

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

Update draft-ietf-quic-transport.md

e0d76ce

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

Update draft-ietf-quic-transport.md

48f04d6

Co-Authored-By: Martin Thomson <mt@lowentropy.net>

janaiyengar reviewed Nov 1, 2019

View reviewed changes

ianswett and others added 5 commits November 2, 2019 16:46

Update draft-ietf-quic-transport.md

a7232f1

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

Update draft-ietf-quic-transport.md

ec80c86

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

Update draft-ietf-quic-transport.md

cd47d62

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

Update draft-ietf-quic-transport.md

7a6d513

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

Update draft-ietf-quic-transport.md

0bfcf11

Co-Authored-By: Jana Iyengar <jri.ietf@gmail.com>

ianswett mentioned this pull request Nov 2, 2019

Improve the idle timeout text #3184

Closed

ianswett changed the title ~~Idle timeout indicates you will timeout~~ Change to max_idle_timeout Nov 2, 2019

Merge branch 'master' into ianswett-symmetric-timeout

210821b

ianswett changed the title ~~Change to max_idle_timeout~~ Change idle_timeout to max_idle_timeout Nov 2, 2019

marten-seemann mentioned this pull request Nov 3, 2019

use the minimum of the peers' idle timeouts quic-go/quic-go#2194

Closed

Jana's comment

07d63d9

martinthomson approved these changes Nov 3, 2019

View reviewed changes

marten-seemann reviewed Nov 4, 2019

View reviewed changes

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

ianswett added 3 commits November 18, 2019 08:29

Update draft-ietf-quic-transport.md

e74856d

Marten's suggestion

581a368

Rebuild

875de3f

martinthomson reviewed Dec 11, 2019

View reviewed changes

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

draft-ietf-quic-transport.md Outdated Show resolved Hide resolved

janaiyengar reviewed Dec 11, 2019

View reviewed changes

martinthomson added 2 commits December 11, 2019 08:06

Use idle timer more directly

774bc68

wrap

c559540

huitema mentioned this pull request Jan 17, 2020

Prepare draft 25 private-octopus/picoquic#772

Closed

martinthomson added the design An issue that affects the design of the protocol; resolution requires consensus. label Jan 21, 2020

Merge branch 'master' into ianswett-symmetric-timeout

a203af6

martinthomson merged commit 2432593 into master Jan 21, 2020

martinthomson deleted the ianswett-symmetric-timeout branch January 21, 2020 05:45

ianswett added a commit that referenced this pull request Feb 9, 2020

Idle timeout editorial fixes

f247379

From @janaiyengar and @MikeBishop comments on #3099 Fixes #3184

ianswett mentioned this pull request Feb 9, 2020

Idle timeout editorial fixes #3444

Merged

marten-seemann mentioned this pull request Mar 13, 2020

rename idle_timeout to max_idle_timeout quicwg/qlog#63

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change idle_timeout to max_idle_timeout #3099

Change idle_timeout to max_idle_timeout #3099

ianswett commented Oct 16, 2019 •

edited

janaiyengar Oct 16, 2019

ianswett Oct 16, 2019

martinthomson left a comment

kazuho left a comment

martinthomson left a comment

MikeBishop Oct 30, 2019

ianswett Nov 2, 2019

janaiyengar left a comment

janaiyengar Nov 1, 2019

janaiyengar Nov 1, 2019

ianswett Nov 2, 2019

janaiyengar Nov 1, 2019

ianswett Nov 2, 2019

janaiyengar Nov 5, 2019

ianswett Nov 20, 2019

ianswett commented Nov 2, 2019

martinthomson Nov 3, 2019

ianswett commented Nov 20, 2019

janaiyengar left a comment

janaiyengar Dec 11, 2019

Change idle_timeout to max_idle_timeout #3099

Change idle_timeout to max_idle_timeout #3099

Conversation

ianswett commented Oct 16, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

kazuho left a comment

Choose a reason for hiding this comment

martinthomson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janaiyengar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett commented Nov 2, 2019

Choose a reason for hiding this comment

ianswett commented Nov 20, 2019

janaiyengar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianswett commented Oct 16, 2019 •

edited