Some notifications are lost when using Push.payloads() with a large number of threads

[GoogleCodeExporter]

What steps will reproduce the problem?

1. The problem is not very easy to reproduce but I noticed during tests that 
when sending a simple payload to a lot of devices (more then 10000) using 20 
threads for example, the first device sometimes did not receive the 
notification. In the log there are no errors regarding this device.

To perform the test, I generate a list of 9998 fake devices and add 2 real 
devices at the beginning and end.

What is the expected output? What do you see instead?
I expect to receive the notification on my 2 devices but most of the time the 
first device never receives it. 

What version of the product are you using? On what operating system?

2.2 beta 3

Please provide any additional information below.

After many tests, I got better results after adding a 'startup delay' (around 
500 ms) in the Notifications.start() method. This avoids a peak of simultaneous 
 connections to the apple servers (when the pool of threads is starting), which 
causes some notifications to be lost.

here is the code:

public synchronized NotificationThreads start() {
        if (started)
            return this;
        started = true;
        if (threadsRunning > 0)
            throw new IllegalStateException("NotificationThreads already started (" + threadsRunning
                + " still running)");
        assignThreadsNumbers();
        for (NotificationThread thread : threads) {
            threadsRunning++;
            thread.start();
            if (threadStartupDelay > 0) {
                try {
                    Thread.sleep(threadStartupDelay);
                } catch (InterruptedException e) {
                }
            }
        }
        if (listener != null)
            listener.eventAllThreadsStarted(this);
        return this;
    }

It is more a workaround than a fix, but I think it's worth to have this feature 
in the next release. 

Original issue reported on code.google.com by ericpr...@gmail.com on 19 Jan 2012 at 9:29

[GoogleCodeExporter]

Could you attach the source code for your test please?

Original comment by sype...@gmail.com on 19 Jan 2012 at 3:01

[GoogleCodeExporter]

Also, you indicated you were using 2.2 Beta 3, but I am assuming you are really 
using Beta 2?

Original comment by sype...@gmail.com on 19 Jan 2012 at 3:35

[GoogleCodeExporter]

yes beta 2 sorry.
For the code I use your test example 
NotificationTest.pushSimplePayloadUsingThreads() with 10000 devices and 50 
threads. I add 2 real devices to the generated fake devices (1 at the beginning 
and 1 at the end).

Original comment by ericpr...@gmail.com on 19 Jan 2012 at 3:47

[GoogleCodeExporter]

With that many threads and devices, are you absolutely sure that there is no 
error in the hundreds of thousands of lines of debug output generated?

Also, is it always the first (or the last) device that fails to receive the 
notification, or is it sometimes the first, sometimes the last?

Original comment by sype...@gmail.com on 19 Jan 2012 at 4:05

[GoogleCodeExporter]

When I did the tests, I put the logger in ERROR, to reduce the logs generated 
by javapns. I have also modified the test to display only failed 
PushedNotifications.

In the logs I did not see any error regarding my 2 real devices, but I did not 
check all the time.

It is a very odd behaviour, most of the time, the first device fails to receive 
the notification. Sometimes the last but less often.

What I have noticed is that after adding this startup delay, I got better 
results.

Original comment by ericpr...@gmail.com on 19 Jan 2012 at 5:22

[GoogleCodeExporter]

I have committed r352 to the trunk, which introduces a 500ms delay between 
thread startups to improve reliability, as per your suggestion.

Since the test itself is somewhat unrealistic (pushing to 10,000 invalid 
tokens), I would be tempted to put this issue in low priority, or even closing 
it since I believe we've made our best efforts to improve reliability on such a 
difficult test.  Will think about this further, comments are welcome...

Original comment by sype...@gmail.com on 22 Jan 2012 at 1:08

• Changed title: Some notifications are lost when using Push.payloads() with a large number of threads
• Changed state: Accepted

[GoogleCodeExporter]

Thanks a lot,
Just a suggestion regarding the fix implementation. Is it possible to have it 
configureable (via a setter) like 'sleepBetweenNotifications' in 
NotificationThread ?

Thanks again for your help,
Regards,
Eric

Original comment by ericpr...@gmail.com on 23 Jan 2012 at 9:26

[GoogleCodeExporter]

Committed r353 and released 2.2 Beta 3 with a getter/setter pair in 
NotificationThreads to customize the delay.

Closing as planned.

Original comment by sype...@gmail.com on 23 Jan 2012 at 4:38

• Changed state: Fixed