Apple Push Notification Service implementation - pitch

In APNSConnnection, is it safe to share a single HTTP2StreamMultiplexer between channels?

The queue is for pipelining so you can do

let res1 = conn.send(req)
let res2 = conn.send(req2)

res1.wait()
res2.wait()

Not sure I fully understand that but channelInitializer should only be called once.

We're waiting for a better way of handling that upstream in NIO, but for now that's the way we got it to work based on example code and @tanner0101 chatting with @lukasa

Hm I thought that the preferred way of doing HTTP/2 is to use a new client stream per request. So no pipelining on a single stream. In fact this is exactly what you are doing, that is why I don’t think you need the array.

Ah yes you are right, because you don’t reuse the bootstrap. If the api would allow to reuse the bootstrap to create multiple connections to apns, only then it would be a problem.

1 Like

Are you testing things out or just reviewing? Hoping someone else starts testing this besides me!
:wink:

@t089 ah, I think you are right actually. Good catch. We call HTTP2StreamMultiplexer.createStreamChannel on each APNSConnection.send call anyway, so there is no need for the stream handler to have an array. The HTTP2 multiplexer takes care of pipelining. We should be able to get away with just storing an optional there, although the array doesn't necessarily hurt.

Fair point, I‘ll try to give it proper test run next week. :slight_smile:

1 Like

In a ClientBootstrap, the channelInitializer can be invoked multiple times (because of happy eyeballs racing resolution) but all but one of these Channels will be closed by NIO itself. Also check the documentation. We will improve this but not right now.

Sorry, only skimmed the thread here but couldn't quite figure out what exactly you're waiting for NIO do improve on.

In HTTP/2, the only way is to use a fresh stream per request. HTTP/2 doesn't allow for multiple requests be sent through one stream. Note however that one connection can have many streams in HTTP/2 so you can do many many requests in one connection, each using their own stream (that solves the head of line blocking from HTTP/1.1).

Note that NIO organises this the following way:

                                      /------ another HTTP/2 stream, a Channel [c1]
                                     /--- one HTTP/2 stream, also a Channel [c2]
--- network connection, a Channel [c]
                                     \--- yet another HTTP/2 stream, a Channel [c3]
                                      \----- and a final HTTP/2 stream, also a Channel [c4]

So both the actual network connection (named [c] above) as well as all the HTTP/2 streams (named cN above) are all Channels. The 'child channels' will have channel.parentChannel set to the 'parent channel' which is the actual network connection. Example: c3.parentChannel == c.

Hope that makes sense.


Feel free to skip the rest because this is not relevant for APNS because HTTP/2 doesn't do pipelining but the much better multiplexing instead.

But because it was discussed above: If you have a protocol that doesn't do multiplexing (HTTP/1.1, many database protocols, redis, ...) then swift-nio-extras contains a great helper: RequestResponseHandler which solves a problem that many are running into providing functions like

func send(request: MyRequest) -> EventLoopFuture<MyResponse>

essentially what you're doing here is terminating the ChannelPipeline and adapting a request/response model on top of futures. If you support pipelining you will then always need to hold an array (or better CircularBuffer) of promises to be fulfilled when you receive the response.

But there's no point for repeating this over and over again, so with this

someChannel.pipeline.addHandler(RequestResponseHandler<MyRequest, MyResponse>()) // needs to be the last handler

you will be able to write

// send a request and a fresh promise for the result
func send(request: MyRequest) -> EventLoopFuture<MyResponse> {
    let promise = self.channel.makePromise(of: MyResponse.self)
    self.channel.writeAndFlush((MyRequest(...), promise))
    return promise.futureResult
}

and everything will be handled automatically. The request will be sent pipelined straight away and when the corresponding response comes in, the promise will be fulfilled. So I'd recommend do adopt this for all pipelined protocols where you have a func send(request: MyRequest) -> EventLoopFuture<MyResponse> on your connection object which holds the Channel.

1 Like

Ill have to defer to @tanner0101 here because they had the discussion with @lukasa, but my understanding was that there would be a better way to client bootstrap for HTTP2 in the future, or it would be some improvement to the client bootstrap itself.

I can't tell you which though cause I didn't have that conversation :frowning:

There are two issues that I think we need to address here.

The first is that NIO has established the "bootstrap" as the primary configuration notion for new connections, whether that is inbound or outbound. However, NIO provides no way to say what a bootstrap actually is. NIO has 3 bootstraps in the core, and 2 bootstraps in swift-nio-transport-services, and while all of those have bootstrap in the name and do roughly similar jobs, there is no core common thing that all these bootstraps do.

Anyone who wants to write code that can "set up a connection" without worrying about exactly what that looks like needs to define their own protocol that indicates what they need and then conform the bootstraps they want to it. This isn't exactly ideal, and in the NIO 2 cycle I want us to put some time into defining a common notion of what a "bootstrap" really is, and work on users being able to write code that can operate generically on bootstraps.

The second problem is that HTTP2StreamMultiplexer, from some perspective, looks a lot like a bootstrap. In fact, it looks like a bootstrap and a bootstrap factory: it has a channelInitializer for inbound streams, and createStreamChannel which accepts a channelInitializer for outbound streams. This is pretty awkward, because it sorta walks and quacks a bit like a bootstrap, but without 100% buying into the idea.

So I think we should grow a wrapper around this type that actually gives you a bootstrap that conforms to the above notion of what a bootstrap is.

To be clear, however, none of this is likely to happen imminently: we're still a decent distance off from that. You gotta crawl before you can walk.

Hey Cory, thanks for clarifying.

I presume now I play the waiting game to move this from pitch to proposal as there a few pitches ahead of this one.

Still accepting feedback though, and working on connection pooling/apns-kit here.

1 Like

Hi @kylebrowning, I think that you could probably start getting the proposal ready, obviously at your own time, no rush. I think this could be useful because the SSWG proposals so far have gotten quite a bit of feedback that we always had a 'proposal discussion' thread, followed by a 'proposal feedback' thread after feedback has been incorporated. Where we are right now is:

  • logging, has been accepted and is in sandbox, soon to be tagged
  • metrics, has been accepted and is in sandbox
  • postgres has had its 'proposal discussion' thread and should soon open the 'proposal feedback' thread
  • redis it currently in its 'proposal discussion' thread but I'd soon expect a 'proposal feedback' once we find a good 'batching API'

In other words, in two to three weeks, I'd expect postgres to be voted on, and Redis getting close to a vote. So I think there's space for this one to enter the 'proposal discussion'. But as I say, in your own time. It usually also takes a bit of time to write up the whole proposal.

Let me know what you think.

1 Like

Thanks for this update!

One thing I haven't gotten to is tests. Will that affect the proposals votes, or just its maturity level once voted on?

Ill use other proposals to model this one and then I just submit it to the proposal section?

Hi Kyle,

We wrote some "minimal requirements" here: sswg/incubation.md at master · swift-server/sswg · GitHub

Unit tests are one of those so it would be good to have some coverage in place.

Yes you are very welcome to base your proposal on the structure of the existing ones.

1 Like

From precedence so far and the spelling out of the incubation process, yes. It only affects the stage at which it is accepted as it is an indicator of maturity and health of a package.

Ian is correct, I recalled incorrectly from memory on the minimum requirements.

Yup, so far the SSWG has had the authors choose to post the discussion thread on their own time (since we have to find time to author the proposal and have the package in a shape for the discussion :slight_smile:).

When it's time for the review thread, the SSWG will make the post to officially kick it off.

IMHO you don't need to match this, but the SSWG has accepted a template format for the documented proposal. Tanner chose to essentially copy/paste his to the discussion thread, while I chose to post a subset of mine.

1 Like

Hey everyone, the proposal discussion thread is now open.

1 Like

@tomerd Can you lock this one too, thanks!