-
Notifications
You must be signed in to change notification settings - Fork 226
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
Token Feedback (protocol version2) + Blacklist xmlrpc methods #31
Conversation
Invalid APNS tokens i.e. sandbox tokens will cause Apple to drop the APNS SSL connection. By using the extended protocol we can provide an ID with each message and Apple will send a binary notice prior closing the connection with the token causing the problem and a reason code. Tokens are stored in a "dict buffer" for later lookup. If a token causes a connection drop, the token will be added to a global black list (per appID). Next time the token will be filtered out. This will at least provide a level of "self-healing" - Next step would be to expose the black list via XML-RPC so that the client can fetch it (like the feedback service data)
It would be nice if the xmlrpc methods could be separated in the way I did it in this branch: https://github.com/samuraisam/pyapns/blob/rest-service/pyapns/server.py#L471 and https://github.com/samuraisam/pyapns/blob/rest-service/pyapns/server.py#L631 That way users can use either version as necessary and lessens the possibility of breaking existing installations. |
I wasn't aware that you already have an enhanced protocol branch - even with REST - nice. I agree that adding additional methods instead of modifying existing ones is way less intrusive. From my perspective it may make sense to just close both requests and use the rest-servie branch as a basis to include the blacklisting code |
I'm planning to use pyapns in production soon, but we need support for v2 of the protocol and blacklisting tokens on the server. I'd rather use what you've got than re-invent the wheel and add it on myself. What's the best branch or clone repo to start with to pull in these features as of now? |
@gregbayer use rest-service branch. Barring the REST service implementation (which is incomplete), the version 2 of the protocol is ready, and I have been using it in production for over a year. |
Thanks! Will try it out. Is there any implementation of immediate blacklisting of bad tokens as @endyman suggested in his pull requests? Or have you found that the feedback service is fast enough for notifying you of bad tokens? We are generally concerned about pushes not going through because of bad tokens and would love a way to immediately identify these and even retry the rest of the messages that did not go through in that block. Resending the rest of the tokens that weren't bad from a failed request doesn't happen in the REST service branch yet, does it? Also, I noticed you have a new 'log_disconnections' flag. But it's not available via the client yet? Thanks for your help. |
The way I've been doing it is to just send one notification at a time using notify2. The only case in which you will have a block fail due to bad tokens is if you are sending bulk notifications over the xmlrpc connection. disconnection_log is a part of the rest-service branch. Call You can poll the disconnection log to get a list of responses from apple that occurred at the time of a disconnection. It's helpful for debugging errors in your own programming (too large of a payload, etc): The errors are: 0: No errors encountered In my python app I just log these to Sentry every now again, though almost never see any. |
Cool. Do you have any scalability concerns regarding only sending one notification at a time using notify2? Have you tried to do any load testing of this approach? I know Instagram had a good experience using pyapns on just one server, but I assumed they were sending notifications in batches - so I was doing that too. My intuition says that switching to one at a time might require multiple pyapns servers at Instagram scale. What do you think? |
pyapns is a twisted xmlrpc service and would be the primary bottleneck (the connection to apple would not). That being said, I can't speak for instagram, but your code is just going to be simpler if you send one at a time, even if you find you need extra servers. Assuming you have some form of network monitoring, just keep an eye on latency to the pyapns server, and if that starts to go up, you might want to load balance between two or more. Also you could run on pypy which would give you a ~2-5x speed improvement if it becomes an issue. In my application I just push messages onto a queue that celery picks off and sends to pyapns. I've not experienced any bottleneck yet and have sent a LOT of notifications (high throughput) this way for several years. |
thanks! Sounds reasonable. I'll give it a try. |
Also, if you have a chance, it would be great to merge that into master. I am the only one I know to be running it in production and would like to merge it in once there is someone else also on board. |
I'm looking for a little clarification of the status of the branches. @samuraisam it sounds like you're running in production off of the rest-service branch, but it doesn't currently seem to compile. @endyman's blacklist_xmlrpc appears to be the only place to get a working v2 impl. Is this correct? Any chance of notify2 showing up in master? |
Hey @troppoli I will try to get this in soon. The version I'm using in production at Lightt is the current master head. I have some time next week and I'll try to merge in this functionality. |
cool, thx |
@troppoli Sorry for not sending a pull request sooner. Based on @samuraisam's suggestion, we have been running the rest-service branch at Pulse for a while. We had to make these changes: #39 I meant to try merging in into master, but never go around to it. Let me know if you have any questions. |
This is the more solid version of the token feedback patch.
It also includes XMLRPC methods for server and clients to fetch the blacklist (which will be cleared afterwards) in order to deprovision the tokens.