Optionally close connections after a set number of send attempts #118
Conversation
…es/pushy into close_after_send_attempt_limit Conflicts: src/main/java/com/relayrides/pushy/apns/ApnsConnection.java src/main/java/com/relayrides/pushy/apns/ApnsConnectionConfiguration.java
…es/pushy into close_after_send_attempt_limit
…es/pushy into close_after_send_attempt_limit
|
|
||
| if (this.configuration.getSendAttemptLimit() != null && ++this.sendAttempts >= this.configuration.getSendAttemptLimit()) { | ||
| log.debug("{} reached send attempt limit and will shut down gracefully.", this.name); | ||
| this.shutdownGracefully(); |
There was a problem hiding this comment.
Why do we want to shutdown gracefully if a failure notification has already been sent? Should this parallel #117 and shutdown immediately if the failure notification has already been sent?
There was a problem hiding this comment.
I'm not following; we're not really considering whether we've sent a known-bad notification at this point. It's true that we're aware of it in the else block at line 556, but that's just for deciding if we should attempt to write the maybe-good notification the caller was trying to send.
In any case, even if a known-bad notification has been sent, we don't want to force a shutdown (before the timeout, anyhow) because we might still be waiting for the APNs gateway to respond to our known-bad notification. Make sense? I'm worried that I'm missing your point.
There was a problem hiding this comment.
Discussed offline: The changes in #117 will take care of the shutdown gracefully timeout case.
…es/pushy into close_after_send_attempt_limit Conflicts: src/main/java/com/relayrides/pushy/apns/ApnsConnectionConfiguration.java src/test/java/com/relayrides/pushy/apns/ApnsConnectionTest.java
…ionConfiguration.
|
@AndreaLev Feeling good about this one? |
| if (this == obj) | ||
| return true; | ||
| if (obj == null) | ||
| return false; | ||
| if (getClass() != obj.getClass()) | ||
| return false; | ||
| final ApnsConnectionConfiguration other = (ApnsConnectionConfiguration) obj; | ||
| ApnsConnectionConfiguration other = (ApnsConnectionConfiguration) obj; |
There was a problem hiding this comment.
Very minor: Why did you remove the final? It doesn't look like you're modifying other
There was a problem hiding this comment.
Oops. Just let Eclipse generate a new equals method, and I guess it must have stripped it. Not intentional. Restored in 2f64f36.
|
👍 |
Optionally close connections after a set number of send attempts
This is (I think) the last in a series of changes intended to work around an APNs design issue where we can't know if connections are still alive. The idea here is to (optionally) close connections after a user-configurable number of notifications have been sent.
While #116 closes connections that sit idle, connections may never become idle in applications that generate lots of push notifications. This change allows very active connections to shut down after a set number of send attempts, so even applications that never allow for idle connections will have a reliable way to cycle connections.
EDIT: Changes are probably best viewed with whitespace-only changes hidden.