Re: Proposal: http2 wire format

From: Damir Simunic <damir(dot)simunic(at)wa-research(dot)ch>
To: Craig Ringer <craig(at)2ndquadrant(dot)com>
Cc: David Fetter <david(at)fetter(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal: http2 wire format
Date: 2018-03-26 09:01:18
Message-ID: 6C5075EE-DA6D-4F38-B9FC-8357B1E4D70A@wa-research.ch
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

> On 26 Mar 2018, at 05:11, Craig Ringer <craig(at)2ndquadrant(dot)com> wrote:
>
> On 26 March 2018 at 06:00, Damir Simunic <damir(dot)simunic(at)wa-research(dot)ch> wrote:
>
> > - Overhead for all clients. It may be tiny, but it needs to be
> > measured and that cost needs to be weighed against the benefits.
> > Maybe a cache miss in the context of a network connection is
> > negligible, but we do need to know.
>
> Important point. If h2 is to be seriously considered, then it must be an improvement in absolutely every aspect.
>
> The core part of this proposal is that h2 is parallel to v3. Something one can opt into by compiling `--with_http2`.
>
> IMO, a new protocol intended to supersede an old one must be a core, non-optional feature. It won't reach critical mass of adoption if people can't reasonably rely on it being there. There'll still be a multi-year lead time as versions that support it become widespread enough to interest non-libpq-based driver authors.

Agreed, it should be in core.

>
> My PoC strategy is to touch existing code as little as possible. Yet if the ProcessStartupPacket can somehow return the consumed bytes back to the TLS lib for negotiation, then there’s zero cost to protocol detection for v2/v3 clients and only h2 clients pay the price of the extra check.
>
> As others have noted, you'll want to find a way to handle this in the least SSL-implementation-specific manner possible. IMO if it can't work with OpenSSL, Windows's SSL implementation and OS X's SSL framework it's a non-starter.

Understood.

Everyone that matters supports ALPN: https://en.wikipedia.org/wiki/Application-Layer_Protocol_Negotiation#Support

From the PoC standpoint, it’s now a straightforward chore to make sure it is supported for all possible build choices.

>
> > - Dependency on a new external library. Fortunately, it's MIT
> > licensed, so it's PostgreSQL compatible, but what happens if it
> > becomes unmaintained? This has happened a couple of times, and it
> > causes overhead that needs to be taken into account.
>
> I chose nghttp because it gave me a quick start, it’s well designed, a good fit for this kind of work, and fortunately indeed, the license is compatible. (Also, curl links to it as well, so am pretty confident it’ll be around). Very possible that over time h2 parsing code migrates into pg codebase. There are so much similarities to v3 architecture, we might find a way to generalize both into a single codebase. Then h2 frame parser/state machine becomes only a handful of .c files.
>
> h2 is a standard; however you decide to parse it, your code will eventually converge to a stable state in the same manner that febe v3 code did. Once we master the protocol, I don’t think there’ll be much need to touch the framing code. IOW even if we just import what we need, it won’t be a big issue.
>
> While I'm a big fan of code reuse and using existing libraries, I understand others' hesitance here. Look at what happened with ossp-uuid; that was painful and it was just a contrib.
>
> It's a difficult balance between NIH and maintaining a stable core.

Enough important projects depend on libnghttp, I don’t think it will go away any time soon. And http2 is big; as more and more tools want to talk that protocol they’ll turn to libnghttp, so the signs of any troubles will be visible very very quickly.

>
>
>
> * Is there merit in the idea of a completely new v4 protocol—one that freezes the v3 and takes a new path?
>
> Likely so... but it has to be pretty compelling IMO. And more importantly, offer a smooth backwards- and forwards-compatible path.
>
>
> * What are the criteria for getting this into the core?
>
> Mine would be:
>
> - No new/separate port required. Works on existing port.
>
Check.

> - Doesn't break old clients connecting to new servers
>
Check.

> - Doesn't break new clients connecting to old servers
>

Old server sends “Invalid startup packet” and closes the connection; client’s TLS layer reports an error. Does that count as not breaking new clients?

curl -v https://localhost:5432

...
* OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to localhost:5432
* stopped the pause stream!
* Closing connection 0
curl: (35) OpenSSL SSL_connect: SSL_ERROR_SYSCALL in connection to localhost:5432

This applies to any TLS client (an h2-supporting libpq-fe will behave the same):

wget -v https://localhost:5432

Connecting to localhost|::1|:5432... connected.
Unable to establish SSL connection.

> - No extra round trips for new client -> old server . I don't personally care about old client -> new server so much, but should be able to offer a pg_hba.conf option to ensure v3 proto only or otherwise prevent extra round trips in this case too.

Can we talk about this more, please?

>
> - Offers significant, concrete benefits and solves the outstanding set of issues with v3 comprehensively

This proposal aims to do exactly that. Work on the existing TODO list items is the way to stay on topic and demonstrate a strong case.

Once we can clear the todo list items, we can of course discuss many other benefits. As soon as I get enough of the framing working, I’ll dive into addressing each TODO item, and then scour the mailing list for more “I wish it could do…” remarks.

>
> - Offers a really strong extensibility path for client-requested and server-requested optional protocol features as well as protocol version negotiation, with no extra round trips whenever possible.
>

Check.

Extensibility is the essence of h2, we’re getting this for free.

> - Has a wireshark dissector

Check.

>
> - Is practical to implement in connection pooler proxies like pgbouncer, pgpool

Something I’m planning to look into and address.

New connection poolers might become feasible, too: nginx, nghttpx, etc. (for non-web related scenarios as well). Opting into h2 lets us benefit from a much larger amount of time and resources being spent on improving things that matter. Reverse proxies face the same architectural challenges as pg-only connection poolers do.

>
> - Can be made wholly transparent to clients of libpq, i.e. no extra headers or libraries to link

Check.

This proposal focuses on changes in framing in the following ways:

* client->server: packaging the startup packet into HEADERS, and optionally sending the query and parameters in a DATA frame.
* server->client: moving response packet tags into HEADERS frames and dropping length prefix. DATA frames still contain the usual v3 payload.

Existing code linking against the new libpq client should not even notice the protocol change.

New code wanting to use more of the h2 benefits will of course have to be written differently. That should be a separate conversation, once h2 is in the core. (I’m talking about new features like feature negotiation, etc.—obviously there will have to be new features supported on the server before there’s anything to negotiate).

>
> - Works on windows and osx too
>

Check.

The plan is to use the existing socket and TLS code that v3 uses. I think I can make it work elegantly through the existing PQcommMethods abstraction.

> - Any libraries used are widespread enough that they're present in at least RHEL7 and Debian Stable. We *can't* just bundle extras in our sources, and packagers are unlikely to be at all happy packaging an extra lib or backport for us. They'll probably just disable the new protocol.

Check.

Let me see if I can make a table showing parallel availability of Postgres and libnghttp versions on mainstream platforms. If there are any gaps, I’m sure it is possible to lobby for inclusion of libnghttp where it matters. I see Debian has it for wheezy, jessie, and sid, while pg10 is on sid and buster.

>
> - No regressions for support of SASL / SCRAM, GSSAPI, TLS with X.509 client certs, various other auth methods.
>

Check.

Adding new auth method keyword (“h2”) in pg_hba will give us a clean code path to work with.

> Now, a protocol that cannot satisfy these is IMO not a complete non-starter. It just has to be treated as an optional feature to help out webapps, with quite different design criteria as a result, and cannot be allowed to be as intrusive. Where changes to core protocol logic paths are required it'd have to add plugin mechanisms/hooks instead of adding its own new logic directly.

While web-related scenarios are the first thing that comes to ming when talking about h2, (and that should not be disregarded), this proposal looks at the bigger picture of future-proofing the protocol. Headers/data/trailers split, and feature/ content negotiation are far bigger benefits then being web friendly.

>
> Make sense?

Exactly what I was looking for, thanks! Hopefully we hear from more folks about the concerns with taking this path.
>
>
> * Is it better to develop in an experimental fork until the architecture is stable and than patch onto the master, or are we supposed to keep proposing patches for inclusion in the master? Even if not all details are fully fleshed out?
>
>
> Protocol support doesn't change fast.
>
> I strongly advise you to work on git master at all times, and become familiar with:
>
> - git rebase
> - git cherry-pick
> - git merge
> - git reflog (for when you make mistakes with the above)
>
> Consider maintaining a public git repo with the current working branch. Tag versions if you refer to them in mailing list posts etc, so that people know the exact code you were referring to.

Will do.

Thanks,
Damir

>
> --
> Craig Ringer http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2018-03-26 09:02:22 Re: Problem while setting the fpw with SIGHUP
Previous Message Dmitry Ivanov 2018-03-26 08:51:42 Re: new function for tsquery creartion