search

mwkirk / javapns

Test import of svn javapns repo from Google Code
3 stars 0 forks source link

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

Closed mwkirk closed 11 years ago

mwkirk commented 11 years ago

Original author: ericpr...@gmail.com (January 19, 2012 09:29:34)

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

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

Original issue: http://code.google.com/p/javapns/issues/detail?id=102

mwkirk commented 11 years ago

From sype...@gmail.com on January 19, 2012 15:01:32 Could you attach the source code for your test please?

mwkirk commented 11 years ago

From sype...@gmail.com on January 19, 2012 15:35:42 Also, you indicated you were using 2.2 Beta 3, but I am assuming you are really using Beta 2?

mwkirk commented 11 years ago

From ericpr...@gmail.com on January 19, 2012 15:47:28 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).

mwkirk commented 11 years ago

From sype...@gmail.com on January 19, 2012 16:05:39 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?

mwkirk commented 11 years ago

From ericpr...@gmail.com on January 19, 2012 17:22:18 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.

mwkirk commented 11 years ago

From sype...@gmail.com on January 22, 2012 01:08:46 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...

mwkirk commented 11 years ago

From ericpr...@gmail.com on January 23, 2012 09:26:07 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

mwkirk commented 11 years ago

From sype...@gmail.com on January 23, 2012 16:38:15 Committed r353 and released 2.2 Beta 3 with a getter/setter pair in NotificationThreads to customize the delay.

Closing as planned.