Skip to content
This repository has been archived by the owner on May 29, 2021. It is now read-only.

Investigate background session type for URLSession #7

Closed
garvankeeley opened this issue Sep 28, 2017 · 14 comments
Closed

Investigate background session type for URLSession #7

garvankeeley opened this issue Sep 28, 2017 · 14 comments

Comments

@garvankeeley
Copy link
Contributor

garvankeeley commented Sep 28, 2017

Test scenario is having a local webserver and my iPhone 5 SE.

Using URLSessionConfiguration.default vs URLSessionConfiguration.background(withIdentifier:) does not behave differently. Both upload the ping within 3 sec after backgrounding the app.
My conclusion is this is not causing missing pings. (Although it could contribute because of the way pings are deleted from disk, and then uploaded, and delays in this loop could increase the chance of a problem here.)

URLSessionConfiguration.background has a few qualities that make it slightly less desirable:

  • it reboots the app to call the completion handler
  • we upload pings in a sequential loop, and Apple's session queue for this will delay each upload increasingly (todo: re-read docs find the reference to this)
  • the design is more targeted towards larger network tasks, these ping files are a few KB in most cases, and under normal network conditions uploading takes a few seconds.
  • harder to test, default URLSession can be stubbed for XCTest.

As discussed with @justindarc the session will get switched to default type.

@garvankeeley
Copy link
Contributor Author

The test app is modified, I need to push it up to the repo.

@garvankeeley
Copy link
Contributor Author

Increasing upload delays is mentioned here, it references a post from Apple for which the link isn't working: https://krumelur.me/2015/11/25/ios-background-transfer-what-about-uploads/

@farhanpatel
Copy link
Contributor

I think it be interesting to look at simply caching the ping and then firing the ping the next time the app launches. Pings are timestamped so when they arrive on the server is usually not an issue.

I know Sentry and some other analytic tools work like this.

@farhanpatel
Copy link
Contributor

Adjust is also open source. so we could look at how they caclulate numbers. And try to build our telemetry in a similar way. Would that work?
https://github.com/adjust/ios_sdk

@justindarc
Copy link
Contributor

We do cache the pings already, however we don't re-try until we go to background the next time. I think we should avoid sending in the foreground if possible, but we can try that if all else fails.

@justindarc
Copy link
Contributor

As far as the calculations go, that's all done server-side and I believe the logic is shared across desktop and other products.

@garvankeeley
Copy link
Contributor Author

In my understanding, it won't make a difference whether we do this on background or foreground.

@Sdaswani
Copy link

In my experience doing it in the foreground is much more reliable. Why should we avoid doing this in the foreground @justindarc ? Is the ping fat?

@justindarc
Copy link
Contributor

@Sdaswani not necessarily. Just thinking of users on a spotty 3G connection that are possibly trying to look something up in Focus. We may not want to be firing off an additional network request at startup no matter how small.

@Sdaswani
Copy link

I wonder if we can be a bit more opportunistic and check if the user is on WiFi and if so then send the ping upon startup. We can then even wait to send the ping until after the first web page is loaded.

@garvankeeley
Copy link
Contributor Author

See #12, this library should only be sending a few KB of data, with hard limits on data volumes. If that isn't possible to implement we should only be sending on wifi.
There could be data loss by restricting to wifi-only from those with cellular data-only internet connections.

@Sdaswani
Copy link

Sorry I didn't want to restrict to WiFi only - if the user is on WiFi I just want to be more aggressive about sending during a session.

But I agree - hopefully the pings are small so we can be aggressive on both WiFi and cellular.

@justindarc
Copy link
Contributor

We talked about this a little bit ago. I don't think we have any reason to believe that the pings get large. We also already have limits on the number of events that we can include in a single ping which should prevent them from getting too large in the first place.

@garvankeeley
Copy link
Contributor Author

Closing as we are seeing no problems with the latest approach on dev branch.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants