tboyko / apple_shove

Multi-Certificate APN Service Provider. More powerful than Apple Push...
MIT License
17 stars 7 forks source link

Allow more connections per certificate #7

Open mmacvicar opened 9 years ago

mmacvicar commented 9 years ago

In the description it is told that it allows notifications to be sent in parallel and that it doesn't wait for a series of connections and disconnections to take place before notification #n could be sent. However, it seems that there is only one actor per certificate that executes a send in exclusive mode, making it parallel only between connections with different certificates.

Do you guys see any problem in using a pool of NotifyConnection for each certificate? Seems like an easy solution and it would allow for higher rates.

tboyko commented 9 years ago

Apple mentions that they have some limitations on how many parallel connections per certificate (or per host? not well defined) they want you to have open at once. If the library was restructured as you are suggesting, the design would need to take these limits into consideration.

On Mar 26, 2015, at 10:11 AM, Michael Mac-Vicar notifications@github.com wrote:

In the description it is told that it allows notifications to be sent in parallel and that it doesn't wait for a series of connections and disconnections to take place before notification #n could be sent. However, it seems that there is only one actor per certificate that executes a send in exclusive mode, making it parallel only between connections with different certificates.

Do you guys see any problem in using a pool of NotifyConnection for each certificate? Seems like an easy solution and it would allow for higher rates.

— Reply to this email directly or view it on GitHub https://github.com/tboyko/apple_shove/issues/7.

mmacvicar commented 9 years ago

I agree, it should still allow limiting the total number of connections being opened. We use around 20 connections simultaneously in production and have already increased them to 100 during massive campaigns (need to push ~100mi messages in some hours). I will send the PR later this week.

mmacvicar commented 9 years ago

BTW, I ignore if using 100 connections cause weird problems but at least with 20 we don't see any issues, due to the lack of details in the documentation these things are quite esoteric. With 100 connections we have never felt the problem, we know that messages get to all of our devices during the campaigns but who knows... apple might be dropping messages.

tboyko commented 9 years ago

My recommendation is to make sure you not only limit the total number of connections being opened (for system memory sake) but also have a smaller limit for the number of connections opened on a per-certificate basis.

I’m not actively using this project any longer and so I don’t have a great way of testing PRs. If you can show adequate testing, I’ll merge. Otherwise, you might be better off keeping your own fork and maintaining it for future users.

Thanks!

On Mar 26, 2015, at 10:58 AM, Michael Mac-Vicar notifications@github.com wrote:

I agree, it should still allow limiting the total number of connections being opened. We use around 20 connections simultaneously in production and have already increased them to 100 during massive campaigns (need to push ~100mi messages in some hours). I will send the PR later this week.

— Reply to this email directly or view it on GitHub https://github.com/tboyko/apple_shove/issues/7#issuecomment-86647207.