Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please use multiple streams to speed up iOS push notifications #15

Closed
nathany opened this issue Jun 13, 2016 · 14 comments
Closed

Please use multiple streams to speed up iOS push notifications #15

nathany opened this issue Jun 13, 2016 · 14 comments

Comments

@nathany
Copy link
Contributor

nathany commented Jun 13, 2016

HTTP/2 supports multiple streams, but it seems like Pigeon currently waits for a response before sending another notification. When timing a push of 1,000 notifications it seemed to be taking > 100ms per notification.

By not waiting for the response before the next push, Pigeon could be sending 20+ notifications concurrently, cutting the time down dramatically.

The number of streams is dynamic, so there is no optimal number of concurrent notifications to send (unfortunately).

The APNs server allows multiple concurrent streams for each connection. The exact number of streams is based on server load, so do not assume a specific number of streams. https://developer.apple.com/library/ios/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/Chapters/APNsProviderAPI.html

@hpopp
Copy link
Member

hpopp commented Jun 13, 2016

Funny you bring this up, this is the last major thing to address before v1.0. I'm still deciding on the best way to go about it-- either implementing chatterbox as the HTTP2 client or fully fleshing out the custom one I've already written. My plan is to have something like this in the next release.

@idyll
Copy link

idyll commented Jun 14, 2016

That's fantastic. If you need help testing/benchmarking, let us know. I'd be super interested in your thoughts on chatterbox for this, even if you don't decide to use it.

@hpopp
Copy link
Member

hpopp commented Jun 16, 2016

I'm thoroughly impressed with chatterbox. Didn't take very long to get it working, and it handles async responses like a champ. It also made fetching the apns-id from the response headers trivial, so I went ahead and added support for it in the APNS.Notification struct.

I pushed a branch of the new changes to play with (not refactored in the slightest). I still haven't read through chatterbox's source thoroughly enough to understand how it behaves with concurrent stream ids, but I'm guessing it should play nice with Apple's servers. From my basic testing so far I've had no problems sending pushes and getting proper responses.

@nathany
Copy link
Contributor Author

nathany commented Jun 24, 2016

Thanks for working on this. I upgraded to Pigeon 0.8.0 but I'm seeing the following error when running my tests:

** (Mix) Could not start application pigeon: Pigeon.start(:normal, []) returned an error: shutdown: failed to start child: :apns_worker
    ** (EXIT) an exception was raised:
        ** (UndefinedFunctionError) function :h2_client.start_link/3 is undefined (module :h2_client is not available)

Am I missing a dependency?

@nathany
Copy link
Contributor Author

nathany commented Jun 24, 2016

I guess that's Chatterbox -- I don't know how to get it and don't see anything in the readme.

@nathany
Copy link
Contributor Author

nathany commented Jun 24, 2016

{:chatterbox, ">= 0.0.0", git: "git@github.com:joedevivo/chatterbox.git"},

okay. 👍

@nathany
Copy link
Contributor Author

nathany commented Jun 27, 2016

It's pretty speedy now. Thanks Henry.

Since chatterbox isn't on hex and isn't a dependency of Pigeon, it could use some documentation for adding it to mix.exs.

There's no recent tag so I'm just using the latest commit for now:

{:chatterbox, ">= 0.0.0", github: "joedevivo/chatterbox", ref: "a01a71bc2713921a60430522cf7323cfa99ffc0e"},

/cc @joedevivo

@joedevivo
Copy link

There's a 0.3.0 tag that's pretty recent

@hpopp
Copy link
Member

hpopp commented Jun 27, 2016

Does mix not auto install github dependencies or am I missing something here? Chatterbox 0.3.0 is listed in the mix.exs

@nathany
Copy link
Contributor Author

nathany commented Jun 27, 2016

@hpopp: I guess not, as I had to add it to our mix.exs to install it.

Thanks.

@nathany nathany closed this as completed Jun 27, 2016
@Douvi
Copy link

Douvi commented Aug 29, 2016

Sorry to open this issue again, but I got the same error when I try to start a second time the server.

here my conf:

def application do
    [...
     applications:[..., :pigeon]]
  end

defp deps do
    [...
     {:pigeon, "~> 0.9.0"},
     {:chatterbox, "~> 0.3.0-15-g401cef6", github: "joedevivo/chatterbox"},
    ...]
  end

I did not succeed to user "0.3.0"

Steps:
1 - mix deps.clean --all
2 - mix deps.get
3 - mix phoenix.server "Ok the server work perfectly and I can send push on iOS"
4 - control c to kill the server
5 - mix phoenix.server I got the error below

** (Mix) Could not start application pigeon: Pigeon.start(:normal, []) returned an error: shutdown: failed to start child: :apns_worker
    ** (EXIT) an exception was raised:
        ** (MatchError) no match of right hand side value: {:error, :eaddrinuse}
            src/h2_connection.erl:148: :h2_connection.init/1
            (stdlib) gen_fsm.erl:325: :gen_fsm.init_it/6
            (stdlib) proc_lib.erl:247: :proc_lib.init_p_do_apply/3

@nathany
Copy link
Contributor Author

nathany commented Aug 29, 2016

Probably better to open a new issue since this one is closed and might get lost.

eaddrinuse is a slightly different error -- I wonder which address is in use -- did control-c not shutdown cleanly perhaps?

❯ ps aux | grep phoenix

@hpopp
Copy link
Member

hpopp commented Aug 29, 2016

Do you have apns_2197: true in your config?

@Douvi
Copy link

Douvi commented Aug 29, 2016

Ok I will create a new issue and send to you all info I have

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants