Subject: [quicwg/base-drafts] Remove ACKs from draining period (#853)

From: Martin Thomson <[email-masked]>
Date: Tue, 10 Oct 2017 23:25:34 +0000 (UTC)

As agreed at the interim, this removes the ability to acknowledge during the
draining period.  An endpoint can use the APPLICATION_CLOSE/CONNECTION_CLOSE
frame to exit this state early (it's a kind of ACK).

Closes #852.
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/853

-- Commit Summary --

  * Remove ACKs from draining period

-- File Changes --

    M draft-ietf-quic-transport.md (21)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/853.patch
https://github.com/quicwg/base-drafts/pull/853.diff

-- 
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/853

---

From: ianswett <[email-masked]>
Date: Wed, 11 Oct 2017 00:19:03 +0000 (UTC)

ianswett commented on this pull request.

> @@ -1462,11 +1462,14 @@ The draining period persists for three times the current Retransmission Timeout
 can be acknowledged, but no new application data can be sent on the connection.
 
 Different treatment is given to packets that are received while a connection is
-in the draining period depending on how the connection was closed.  In all
-cases, it is possible to acknowledge packets that are received as normal, but
-other reactions might be preferable depending on how the connection was closed.
-An endpoint that is in a draining period MUST NOT send packets containing frames
-other than ACK, PADDING, or CONNECTION_CLOSE.
+in the draining period depending on how the connection was closed.  An endpoint
+that is in a draining period MUST NOT send packets unless they contain a
+CONNECTION_CLOSE or APPLICATION_CLOSE frame.

This looks fine, but what's APPLICATION_CLOSE.  Did I miss that from another PR?

-- 
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/853#pullrequestreview-68463582

---

From: Martin Thomson <[email-masked]>
Date: Wed, 11 Oct 2017 02:41:42 +0000 (UTC)

martinthomson commented on this pull request.

See #722.

-- 
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/853#discussion_r143900359

---

From: ianswett <[email-masked]>
Date: Wed, 11 Oct 2017 13:12:27 +0000 (UTC)

I saw that PR go by, but the description didn't mention a new frame type.  I'm not super opposed, but I definitely wasn't expecting it.

-- 
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/853#discussion_r144005775

---

From: ianswett <[email-masked]>
Date: Wed, 11 Oct 2017 13:14:53 +0000 (UTC)

ianswett approved this pull request.

-- 
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/853#pullrequestreview-68600430

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Oct 2017 15:49:15 -0700

janaiyengar commented on this pull request.

one small comment.

 Different treatment is given to packets that are received while a connection is
-in the draining period depending on how the connection was closed.  In all
-cases, it is possible to acknowledge packets that are received as normal, but
-other reactions might be preferable depending on how the connection was closed.
-An endpoint that is in a draining period MUST NOT send packets containing frames
-other than ACK, PADDING, or CONNECTION_CLOSE.
+in the draining period depending on how the connection was closed.  An endpoint
+that is in a draining period MUST NOT send packets unless they contain a
+CONNECTION_CLOSE or APPLICATION_CLOSE frame.
+
+An endpoint that receives either CONNECTION_CLOSE or APPLICATION_CLOSE while in
+the draining period can terminate the draining period immediately.  An endpoint
+that receives either of these frames can send a packet containing a

can send --> MAY send. That said, I don't get the value of sending a CONNECTION_CLOSE here. Why is this useful?

-- 
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/853#pullrequestreview-69084958

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 22:54:19 +0000 (UTC)

I had a chat with @nibanks at the interim and he was concerned about finding a way to quickly terminate the draining period.  Lingering timers are annoying to manage in testing.

-- 
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/853#discussion_r144430082

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 15:55:00 -0700

@martinthomson pushed 1 commit.

fee38a5  s/can/MAY terminate draining period early

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

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Oct 2017 16:17:48 -0700

I don't follow. This says the endpoint terminates draining immediately. Are you thinking the peer might be in draining? Can you give an example?

-- 
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/853#discussion_r144433399

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 23:35:37 +0000 (UTC)

On error client sends CONNECTION_CLOSE.  It has to wait for 3RTO (or whatever) to cleanup state.  But if the server sends a reciprocal CONNECTION_CLOSE, there is far less need to hang around.

I guess that the concern is that you might get reordering around that close.  Maybe that's the problem and maybe @nibanks will have to work out another way to get test cases to end quickly.

-- 
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/853#discussion_r144436083

---

From: Nick Banks <[email-masked]>
Date: Thu, 12 Oct 2017 23:41:49 +0000 (UTC)

nibanks commented on this pull request.

I just was asking for some way to (optionally) acknowledge that the connection was closed, so that the draining period could be short circuited.

Specifically, in my case, I have client and server running on the same machine, and the tests will usually connect them, do something and then try to cleanly disconnect. With >100 test cases and running them locally every time I build, I want them to be as fast as possible.

-- 
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/853#discussion_r144436939

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 23:47:15 +0000 (UTC)

Yeah, though as a practical matter, terminating the draining period won't work in general.  Maybe - with the allowance for a reciprocal close - you can make some assumptions in your test cases that will allow you to terminate early.

The alternative is to do as I suggested: find a way to shift timers.  I do that for every test framework I build because I hate waiting.  It will make your recovery tests run faster as well.

For now, I'm going to remove the termination clause from this PR.

-- 
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/853#discussion_r144437609

---

From: Nick Banks <[email-masked]>
Date: Thu, 12 Oct 2017 23:48:58 +0000 (UTC)

I'm still unclear why it wouldn't work. What is the reordering problem? Why can't you just use ACKs then (which is what I was doing originally before this PR)?

-- 
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/853#discussion_r144437809

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 23:49:27 +0000 (UTC)

a5c71cb  Remove paragraph about terminating draining period early

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

---

From: Martin Thomson <[email-masked]>
Date: Fri, 13 Oct 2017 00:03:34 +0000 (UTC)

Server sends CONNECTION_CLOSE, client sends CONNECTION_CLOSE.  Server throws away state.  Client packet from before the CONNECTION_CLOSE arrives at server.

The point of the draining period is to have a catcher for the delayed packets that might arrive.  Now, if you don't care (because you stopped listening on the socket, for instance), then that's great and you shorten the draining period to whatever you like.  But if you are writing test cases, then you can skew the clock artificially and retain the benefits of the draining period.

(Maybe we need to make it clear that the draining period is somewhat discretionary on the part of endpoints.  Happy to do that in a followup PR.)

-- 
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/853#discussion_r144439524

---

From: Nick Banks <[email-masked]>
Date: Fri, 13 Oct 2017 00:13:07 +0000 (UTC)

Ok, one follow up question then:

What are you supposed to do with incoming packets while in the draining period? Do you throw them away or indicate them up to the application?

If you are throwing them away, what is the harm it interpreting either an ACK for your CONNECTION_CLOSE or a separate CONNECTION_CLOSE as reason enough to end the local draining period? What other state would be lost?

If you are supposed to indicate them to the application, how is that supposed to work, assuming they are the one that initiated the close in the first place? They would likely be in a blocking close() call.

-- 
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/853#discussion_r144440661

---

From: Martin Thomson <[email-masked]>
Date: Thu, 12 Oct 2017 17:19:59 -0700

Dump them.

There is no harm in doing whatever you like at this point - you have decided that the connection is no more.  You either terminated the connection gracefully (because you decided you had everything you needed) or you detected a fatal error of some kind (including stateless reset).

You can - if you like - end the draining period early, but you have to be willing to accept the consequences.  You might still receive packets for a while.  Meaning that you no longer have something that is allocated to handle them properly, though that might not be a particular problem for your architecture.  In the case where you sent the first CONNECTION_CLOSE, it might be the case that your CONNECTION_CLOSE was lost and needs retransmission.  Not being able to generate another CONNECTION_CLOSE means that you might get a LOT more packets.  A server might have to resort to a stateless reset in that case.

-- 
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/853#discussion_r144441424

---

From: janaiyengar <[email-masked]>
Date: Thu, 12 Oct 2017 18:02:25 -0700

So the point of draining is to hang around to respond to packets that the peer doesn't know have been acknowledged. If we go into a draining period after sending a CONNECTION_CLOSE, then we have to wait until we hear back from the peer. When we hear back from the peer, they've heard us, we've heard them, and it's all good. 

However the side that sends the last CONNECTION_CLOSE (in response to a received CONNECTION_CLOSE) cannot leave because it cannot be sure that the peer heard the last CONNECTION_CLOSE.

There more text that surely needs to be added here, so I say let's get this in and we can work out the details of how to finesse this later. I'll merge this.

-- 
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/853#discussion_r144445716

---

From: janaiyengar <[email-masked]>
Date: Fri, 13 Oct 2017 01:02:31 +0000 (UTC)

Merged #853.

-- 
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/853#event-1291425071

---

From: Nick Banks <[email-masked]>
Date: Fri, 13 Oct 2017 01:04:42 +0000 (UTC)

Thanks a lot for walking me through the issues. The main issue is that if you exit draining too early, you might receive packets destined for the connection afterwards and they might be hard to handle, depending on the implementation, correct? Specifically, that is mostly a server side problem, as on the client side, you likely close the socket and don't have to worry about it any more?

Is that reason enough to remove the optional mechanism for exiting the draining early? Shouldn't implementations that know they can't handle it well, just not exit early?

-- 
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/853#discussion_r144445922

---

From: janaiyengar <[email-masked]>
Date: Fri, 13 Oct 2017 01:08:11 +0000 (UTC)

I think we can have text here that says something like the endpoint SHOULD hang around, which leaves a bit of wiggle room for folks who know what they're doing. I'd rather stipulate the thing that keeps folks safe and have the ones who know what they're doing circumvent the stipulation. Do you want to file an issue for leaving draining early? 

-- 
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/853#discussion_r144446348

---

From: Nick Banks <[email-masked]>
Date: Fri, 13 Oct 2017 01:09:30 +0000 (UTC)

Sure, I will open an issue.

-- 
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/853#discussion_r144446448