Issues with bulk messages in the new aioapns dependency (apns_async)

jazzband / django-push-notifications

Send push notifications to mobile devices through GCM or APNS in Django.

MIT License

2.28k stars 617 forks source link

Issues with bulk messages in the new aioapns dependency (apns_async) #744

Open mikaelengstrom opened 2 days ago

mikaelengstrom commented 2 days ago

Hi, i have been trying out the latest async version at scale (Im sending to 200k+ devices in production) and it does not really work for bulk messaging. Already around sending messages to 100 devices it justs hangs indefinitely and never terminates.

I have been trying to fix the issue, however i have zero experience with the underlying libraries (aioapns, h2, aiohttp) and dont really know how its supposed to work. But i managed to make it somewhat stable by using semaphores limiting the number of paralell requests and raising the connection limit. Implementation can be viewed here: https://github.com/mikaelengstrom/django-push-notifications/commit/9a95da876cb25f64d751450a976061b4863c929e

Unfortunatly, performance take quite a hit when done this way.

Very thankful if someone who knows this stuff better could check it out. The original contributor @pomali maybe? :)

mikaelengstrom commented 1 day ago

I worked a bit further on this and it appears to be related to https://github.com/Fatal1ty/aioapns/issues/41 (which in turn probably is related to another issue in h2). I did manage so send a several hundred thousand push notifications with this retry-logic: https://github.com/mikaelengstrom/django-push-notifications/commit/ad9bd17b7e9b30d10205dbd611ba18540ac462b6

mikaelengstrom commented 16 hours ago

So digging deeper, one of the core issues was having non-legit registrationIds (such as BadDevice) in the device-list, making the connection overload and crash for some reason. But appart from that i had some issues with concurrency and the lib simply never terminating properly. I managed to develop a version where bulk-messaging works fast and performant as long as there are few or none non-legit registrationids: https://github.com/mikaelengstrom/django-push-notifications/blob/master/push_notifications/apns_async.py

This update does these main things:

Make sure everything regarding to aioapns happens in an async context and by doing so getting rid of the asyncio.get_eventloop-try-catch and instead just going for asyncio.run()
Setting devices to inactive on the failcodes "BadDeviceToken" and "DeviceTokenNotForTopic"
Integrated uvloop for blazing fast delivery

Any thoughts on this or should i just create a PR?