Re: [quicwg/base-drafts] Clients can send GOAWAY too (#3129)

Martin Thomson <notifications@github.com> Wed, 29 January 2020 09:01 UTC

Return-Path: <noreply@github.com>
X-Original-To: quic-issues@ietfa.amsl.com
Delivered-To: quic-issues@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1CE9B120251 for <quic-issues@ietfa.amsl.com>; Wed, 29 Jan 2020 01:01:16 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3
X-Spam-Level:
X-Spam-Status: No, score=-3 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=0.001, MAILING_LIST_MULTI=-1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=github.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6-3Qw0KDWH4J for <quic-issues@ietfa.amsl.com>; Wed, 29 Jan 2020 01:01:13 -0800 (PST)
Received: from out-25.smtp.github.com (out-25.smtp.github.com [192.30.252.208]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 51FB1120241 for <quic-issues@ietf.org>; Wed, 29 Jan 2020 01:01:13 -0800 (PST)
Received: from github-lowworker-f144ac1.va3-iad.github.net (github-lowworker-f144ac1.va3-iad.github.net [10.48.16.59]) by smtp.github.com (Postfix) with ESMTP id 4D84B280062 for <quic-issues@ietf.org>; Wed, 29 Jan 2020 01:01:12 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=github.com; s=pf2014; t=1580288472; bh=on0lHAONvShTdSZwWWqfTfyX0BFqYPh0OmWkqYboUog=; h=Date:From:Reply-To:To:Cc:In-Reply-To:References:Subject:List-ID: List-Archive:List-Post:List-Unsubscribe:From; b=n+3m9oJKKX2d9ZWzG5ZNZWG3vRQVZ2DSBnrDdWPmxGeT1TLmpFWeQTPkyjGtnrzqM ct0CXZ9OEQWHqfHxJjpG26tubTjl9C0eq0sStIlsxE0GQfrKMpaNwxjz4dqWiUbKZt r0d1AXfxYKg5ngMJBAlikbZU3CtHIIy70ZHdgmnc=
Date: Wed, 29 Jan 2020 01:01:12 -0800
From: Martin Thomson <notifications@github.com>
Reply-To: quicwg/base-drafts <reply+AFTOJKZ2CCP6H4EBJQJPW3F4HZ6FREVBNHHB44NAVM@reply.github.com>
To: quicwg/base-drafts <base-drafts@noreply.github.com>
Cc: Subscribed <subscribed@noreply.github.com>
Message-ID: <quicwg/base-drafts/pull/3129/review/349942011@github.com>
In-Reply-To: <quicwg/base-drafts/pull/3129@github.com>
References: <quicwg/base-drafts/pull/3129@github.com>
Subject: Re: [quicwg/base-drafts] Clients can send GOAWAY too (#3129)
Mime-Version: 1.0
Content-Type: multipart/alternative; boundary="--==_mimepart_5e3149d844df1_50d23ff0de8cd96c65839"; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: list
X-GitHub-Sender: martinthomson
X-GitHub-Recipient: quic-issues
X-GitHub-Reason: subscribed
X-Auto-Response-Suppress: All
X-GitHub-Recipient-Address: quic-issues@ietf.org
Archived-At: <https://mailarchive.ietf.org/arch/msg/quic-issues/rQvFAFTE5QMGa-N_d-WqimPTmyk>
X-BeenThere: quic-issues@ietf.org
X-Mailman-Version: 2.1.29
List-Id: Notification list for GitHub issues related to the QUIC WG <quic-issues.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/quic-issues/>
List-Post: <mailto:quic-issues@ietf.org>
List-Help: <mailto:quic-issues-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/quic-issues>, <mailto:quic-issues-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 29 Jan 2020 09:01:16 -0000

martinthomson approved this pull request.

A few editorial suggestions, but I can live with this.

(I still think that signaling the push ID isn't entirely necessary, but I can see from this text how it might be used.  And I recognize that avoid asymmetry makes the marginal value of including the push ID easier to accept.)

> +The information in the GOAWAY frame enables a client and server to agree on
+which requests or pushes were accepted prior to the connection shutdown. Upon
+sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these

I think that we need to separate these here.  Requests are not processed, but pushes are merely not accepted.  There is a significant difference.

> +which requests or pushes were accepted prior to the connection shutdown. Upon
+sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these
+requests or pushes MAY cancel them.  Clients MAY retry such requests on a

I'm not sure that "MAY cancel" is the right thing to be saying here.  What you want to say is that these will be no longer serviced by the endpoint that initiates the shutdown, but the underlying transport resources (streams, etc...) will still exist, so disposing of those resources might be helpful.

> +sending a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+{{request-cancellation}} and {{frame-cancel-push}}) any requests or pushes that
+have identifiers greater than or equal to that indicated, in order to clean up
+transport state for the affected streams. The endpoint SHOULD continue to do so
+as more requests or pushes arrive.
+
+Endpoints MUST NOT initiate new requests or promise new pushes on the connection
+after receipt of a GOAWAY frame from the peer.  Clients MAY establish a new
+connection to send additional requests.
+
+Some requests or pushes might already be in transit.  Upon receipt of a GOAWAY
+frame, if the endpoint has already sent requests or promised pushes with an
+identifier greater than or equal to that received in a GOAWAY frame, those
+requests or pushes will not be processed.  The endpoint that initiated these
+requests or pushes MAY cancel them.  Clients MAY retry such requests on a
+different connection.

I would make this a new paragraph rather than say "such requests".  This concept is important and this throwaway sentence obfuscates an important concept.  Consider moving this to the next paragraph.

> -for a client-initiated, bidirectional stream (i.e. 2^62-4 in case of QUIC
-version 1).  This GOAWAY frame signals to the client that shutdown is imminent
-and that initiating further requests is prohibited.  After allowing time for any
-in-flight requests to reach the server, the server can send another GOAWAY frame
-indicating which requests it will accept before the end of the connection. This
-ensures that a connection can be cleanly shut down without causing requests to
-fail.
-
-Once all accepted requests have been processed, the server can permit the
-connection to become idle, or MAY initiate an immediate closure of the
-connection.  An endpoint that completes a graceful shutdown SHOULD use the
-H3_NO_ERROR code when closing the connection.
+when the server closes the connection.  An endpoint MAY send multiple GOAWAY
+frames indicating different identifiers, but MUST NOT increase the identifier
+value they send, since clients might already have retried unprocessed requests
+on another connection.

This uses "endpoint", but this really only applies to clients.

-- 
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/3129#pullrequestreview-349942011