Subject: Re: [quicwg/base-drafts] Improve Idle Timeout text (#3414)

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Mar 2020 15:35:10 -0700

martinthomson commented on this pull request.

>  and its state is discarded when it remains idle for longer than the minimum of
-the max_idle_timeouts (see {{transport-parameter-definitions}}) and three times
-the current Probe Timeout (PTO).
+the max_idle_timeouts and three times the current Probe Timeout (PTO).

```suggestion
the max_idle_timeout values and three times the current Probe Timeout (PTO).
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#pullrequestreview-372367704

---

From: Mike Bishop <[email-masked]>
Date: Tue, 10 Mar 2020 15:37:17 -0700

MikeBishop approved this pull request.

>  close ({{immediate-close}}) if it abandons the connection prior to the effective
 value.
 
-An endpoint restarts its idle timer when a packet from its peer is received
-and processed successfully.  The idle timer is also restarted when sending
-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.  An endpoint might need to send packets to avoid an
-idle timeout if it is unable to send application data due to being blocked on
-flow control limits; see {{flow-control}}.
+An endpoint restarts its idle timer when sending an ack-eliciting packet 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.  An endpoint might need to send ack-eliciting
+packets to avoid an idle timeout if it is unable to send application data due to
+being blocked on flow control limits (see {{flow-control}}) or is application
+limited, but expecting response data.

Should "application-limited" be hyphenated?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#pullrequestreview-372368418

---

From: ianswett <[email-masked]>
Date: Tue, 10 Mar 2020 18:42:49 -0700

@ianswett pushed 1 commit.

436c49bc835e6d60d1119b9ae66f2d4274ff0387  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/527d22c8d075ba0bbab8421e7ddfbf05e807545f..436c49bc835e6d60d1119b9ae66f2d4274ff0387

---

From: ianswett <[email-masked]>
Date: Tue, 10 Mar 2020 18:43:56 -0700

ianswett commented on this pull request.

 value.

It hasn't been so far.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#discussion_r390703365

---

From: Marten Seemann <[email-masked]>
Date: Tue, 10 Mar 2020 22:07:06 -0700

marten-seemann commented on this pull request.

-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
+By announcing a max_idle_timeout, an endpoint commits to initiating an immediate
 close ({{immediate-close}}) if it abandons the connection prior to the effective

After removing the previous sentence, "effective value" is not defined any more.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#pullrequestreview-372477962

---

From: ianswett <[email-masked]>
Date: Tue, 10 Mar 2020 22:09:23 -0700

> +other ack-eliciting frames to test for liveness before sending any data that
+cannot be retried safely.  Note that it is likely that only applications or
+application protocols will know what information can be retried.

I've decided not to define this in this PR.  @nibanks if you'd like to do so in a different PR, feel free.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#discussion_r390748658

---

From: Jana Iyengar <[email-masked]>
Date: Thu, 12 Mar 2020 00:41:00 -0700

janaiyengar commented on this pull request.

a few suggestions

> @@ -2406,33 +2406,30 @@ source address.
 
 ## Idle Timeout {#idle-timeout}
 
-If the idle timeout is enabled by either peer, a connection is silently closed
+If a max_idle_timeout is specified by either peer in transport parameters

```suggestion
If a max_idle_timeout is specified by either peer in its transport parameters
```

> @@ -2406,33 +2406,30 @@ source address.
 
 ## Idle Timeout {#idle-timeout}
 
-If the idle timeout is enabled by either peer, a connection is silently closed
+If a max_idle_timeout is specified by either peer in transport parameters
+(see {{transport-parameter-definitions}}), a connection is silently closed

```suggestion
({{transport-parameter-definitions}}), the connection is silently closed
```

-the max_idle_timeouts (see {{transport-parameter-definitions}}) and three times
-the current Probe Timeout (PTO).
+the max_idle_timeout values and three times the current Probe Timeout (PTO).

```suggestion
both max_idle_timeout values and three times the current Probe Timeout (PTO).
```

```suggestion
Restarting this timer when sending a packet ensures that connections are not
closed after new activity is initiated.  An endpoint might need to send ack-eliciting
```

```suggestion
packets to avoid an idle timeout if it is expecting response data, but does
not have or is unable to send application data.
```

>  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 a Probe Timeout
-(PTO; see Section 6.6 of {{QUIC-RECOVERY}}), it is advisable to test for
-liveness before sending any data that cannot be retried safely.  Note that it
-is likely that only applications or application protocols will know what
-information can be retried.
+(PTO; see Section 6.6 of {{QUIC-RECOVERY}}), it is advisable to send a PING or
+other ack-eliciting frames to test for liveness before sending any data that
+cannot be retried safely.  Note that it is likely that only applications or
+application protocols will know what information can be retried.

This is still a bit hard to read. I'll suggest the following text instead:
```
An endpoint that sends packets close to the effective timeout risks
having them be discarded at the peer, since the peer might enter its
draining state before these packets arrive. An endpoint can send
a PING or another ack-eliciting frame to test the connection for liveness
if the peer could time out soon, such as within a PTO (see Section 6.6 of
{{QUIC-RECOVERY}}). This is especially useful if any available application
data cannot be safely retried. Note that the application determines what data
is safe to retry.
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#pullrequestreview-372367343

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:09:37 -0700

d1f1e30ccffdde01a3b9153da9ee7feadb1012e7  Apply suggestions from code review

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/436c49bc835e6d60d1119b9ae66f2d4274ff0387..d1f1e30ccffdde01a3b9153da9ee7feadb1012e7

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:46:52 -0700

9d45cdcbe07cee82134d6769a4cd42912f69d24c  Jana's suggestion

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/d1f1e30ccffdde01a3b9153da9ee7feadb1012e7..9d45cdcbe07cee82134d6769a4cd42912f69d24c

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:48:22 -0700

Thanks, I reverted this.  I think this was an unintentional change, but this has gone through so many iterations I'm not 100% sure.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#discussion_r391538079

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:49:39 -0700

Thanks, I accepted your suggestion.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#discussion_r391538770

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:51:25 -0700

75692741cfce2964c1082e42b61cdf0a08c06a3d  Restore text Marten noted

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/9d45cdcbe07cee82134d6769a4cd42912f69d24c..75692741cfce2964c1082e42b61cdf0a08c06a3d

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:52:21 -0700

d8429d186d900950205b75fd37f3a0034315338d  reflow to minimize the diff

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/75692741cfce2964c1082e42b61cdf0a08c06a3d..d8429d186d900950205b75fd37f3a0034315338d

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:56:33 -0700

05f1a2a7f30e0470858dbccaa7dbf682fc35a686  Fix long lines

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/d8429d186d900950205b75fd37f3a0034315338d..05f1a2a7f30e0470858dbccaa7dbf682fc35a686

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 03:59:07 -0700

27e6eaab8755c65cbd1f3795f7f49703c7c51471  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/05f1a2a7f30e0470858dbccaa7dbf682fc35a686..27e6eaab8755c65cbd1f3795f7f49703c7c51471

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 04:05:52 -0700

bf8d45c45f4af621dad70d4c8daeeb97479b49d3  Merge branch 'master' into ianswett-ito-jana-mbishop

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/27e6eaab8755c65cbd1f3795f7f49703c7c51471..bf8d45c45f4af621dad70d4c8daeeb97479b49d3

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 04:08:33 -0700

87a59a7885df7ea9b86ab0de56bfb5ff71a614f0  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/bf8d45c45f4af621dad70d4c8daeeb97479b49d3..87a59a7885df7ea9b86ab0de56bfb5ff71a614f0

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 04:09:03 -0700

71f065086824beaf6ddb91a3c67782fe07574ae5  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/87a59a7885df7ea9b86ab0de56bfb5ff71a614f0..71f065086824beaf6ddb91a3c67782fe07574ae5

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Mar 2020 13:54:44 -0700

> -likely that only applications or application protocols will know what
-information can be retried.
+An endpoint restarts its idle timer when a packet from its peer is received and
+processed successfully. An endpoint also restarts its idle timer when sending an
+ack-eliciting packet if no other ack-eliciting packets have been sent since last
+receiving and processing a packet. Restarting this timer when sending a packet
+ensures that connections are not closed after new activity is initiated.
+An endpoint might need to send ack-eliciting packets to avoid an idle timeout
+if it is expecting response data, but does not have or is unable to send
+application data.
+
+An endpoint that sends packets close to the effective timeout risks having
+them be discarded at the peer, since the peer might enter its draining state
+before these packets arrive. An endpoint can send a PING or another
+ack-eliciting frame to test the connection for liveness if the peer could
+time out soon, such as within a PTO (see Section 6.6 of {{QUIC-RECOVERY}}).

```suggestion
time out soon, such as within a PTO; see Section 6.6 of {{QUIC-RECOVERY}}.
```

> -flow control limits; see {{flow-control}}.
-
-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 a Probe Timeout
-(PTO; see Section 6.6 of {{QUIC-RECOVERY}}), it is advisable to probe the path
-with an ack-eliciting packet to ensure the connection is still responsive
-before sending any data that cannot be retried safely.  Note that it is
-likely that only applications or application protocols will know what
-information can be retried.
+An endpoint restarts its idle timer when a packet from its peer is received and
+processed successfully. An endpoint also restarts its idle timer when sending an
+ack-eliciting packet if no other ack-eliciting packets have been sent since last
+receiving and processing a packet. Restarting this timer when sending a packet
+ensures that connections are not closed after new activity is initiated.
+An endpoint might need to send ack-eliciting packets to avoid an idle timeout

```suggestion

An endpoint might need to send ack-eliciting packets to avoid an idle timeout
```

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#pullrequestreview-373893224

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 14:07:02 -0700

d834ea87cee4d9b8c3026b2d0b1bf1b72fe9f8ad  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/71f065086824beaf6ddb91a3c67782fe07574ae5..d834ea87cee4d9b8c3026b2d0b1bf1b72fe9f8ad

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 14:07:19 -0700

d654a2e7305749e69cd2895d59dbc49468a7c286  Update draft-ietf-quic-transport.md

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414/files/d834ea87cee4d9b8c3026b2d0b1bf1b72fe9f8ad..d654a2e7305749e69cd2895d59dbc49468a7c286

---

From: ianswett <[email-masked]>
Date: Thu, 12 Mar 2020 14:08:22 -0700

Thanks @martinthomson  suggestions accepted.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#issuecomment-598418905

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Mar 2020 16:30:44 -0700

Merged #3414 into master.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/3414#event-3124900486