matomo-org / matomo-sdk-ios

Matomo iOS, tvOS and macOS SDK: a Matomo tracker written in Swift
MIT License
388 stars 164 forks source link

fix(dispatch): decrease timer interval to 15 sec #345

Closed chreck closed 4 years ago

chreck commented 4 years ago

https://github.com/matomo-org/matomo-sdk-ios/issues/344

brototyp commented 4 years ago

Hi @chreck, thanks for your pull request!

I just checked and tried to understand it. Maybe you can help me there. As far as I am understanding the problem, according to the SO thread you linked in the ticket, iOS terminates the app if it is running longer than 30 seconds in the background.

The dispatchInterval property only defines how long the Matomo SDK should wait before it tries to send the next batch of events to the server.

I am struggling to understand how changing this is preventing the iOS to kill the app if it runs longer than 30 seconds in the background.

chreck commented 4 years ago

Maybe an example helps to understand the case.

  1. The app is running in background
  2. For 2 minutes it does a high memory load task
  3. After the task is finished it generate a tracking event ,Task X finished'
  4. The matomo dispatcher is started with a timeout of 30 second
  5. iOS started a timer to kill the app in 30 seconds
  6. iOS killed the app after 30 seconds
  7. the tracking event ,Task X finished' could not be sent to matomo server because the app is not running anymore
brototyp commented 4 years ago

Hey @chreck, thanks. That helped to understand the issue in question.

I think there are two underlying problems:

  1. The event is not sent to the server early enough (since it, in the worst case, is dispatched only 30 seconds later). If the app is suspended before the event can be sent to the server, it should just be sent the next time it is brought back to the foreground.

  2. If the app is terminated before it could be brought back to the foreground, the event is lost, since currently events are not persisted per default.

Against case 1, there isn't too much we can do in this situation. Reducing the 30 seconds to 15, can't fix this problem, it just reduces the probability. But it comes with the downside of talking to the network more often (which has an effect on battery) since now events are per default dispatched twice as often. Since this is a public, variable, I would leave it to the user to decide which tradeoff to take.

For case 2, there is a public protocol, called Queue, that can be implemented and overwritten in oder to add persistence when adding an event to the queue. There is no persistent queue implemented yet though. Have a look at this issue for further details.

brototyp commented 4 years ago

I will close this for now because I am not convinced it's the right approach. Additionally the dispatchInterval parameter is a var, so can be changed when using the MatomoSDK.

Feel free to open it again if needed.

brototyp commented 4 years ago

Hi @chreck, I just created this PR. My approach there is to wrap every sending of the events into a background task. Can you try it out and give me some feedback?

chreck commented 4 years ago

@brototyp we use currently the dispatch 15 seconds approach. What about the persisting of the events? I dont think I have time to test your approach, maybe someone else. Sorry.