Send tracks events in batches of no more than 1000

[bjhomer]

When sending tracks events, adds a limit of 1000 events to send per batch.

There are two reasons for this:

• Tracks events are currently being fetched on the main queue, because that's where -[TracksService sendQueuedEvents] is being called when the timer goes off. If we fetch an unbounded number of events, it can result in the main thread hanging if there are many tracks events to send.
• The tracks reporting endpoint will reject batches that are too large. Experimentally, I've seen this happen when I tried to send 100k events in a single batch. This would lead to increasingly large event counts on device, as we continued to gather then without ever being able to send them.

With these changes, the UI is much less likely to hang. On my device, fetching a batch of 1000 events took only 0.03s, which is probably acceptable on the main thread. By contrast, fetching 93,000 events took 2.25s, a clearly noticeable hang. (And yes, we have a user who appears to have over 100,000 unsent events. Not sure how.)

---

• I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

[mokagio]

Thanks @bjhomer . I appreciate how you never shy from diving into Tracks to implement fixes. 🙇‍♂️

Happy for this to land as is because it sounds like it would unlock your work.

One question, though:

Tracks events are currently being fetched on the main queue, because that's where -[TracksService sendQueuedEvents] is being called when the timer goes off.

Have you considered moving the call to a dedicated queue?

Just curious if you found issues with changing queue or if it's something that wasn't considered in the interest of getting the fix out.

[mokagio]

My first reaction seeing this change was "if we are batching, what happens to any other batch at this time? Is this safe or will it lose events? do we need to manage the next batch(es)?"

Correct me if I'm wrong given you're surely fresher on the internal details than I am.

I think this is 100% safe without any logic to get the next batch(es). That's because any remaining even will be picked up on the next sendQueuedEvents call, and that will continue until the even queue will be empty.

[bjhomer]

Yeah, they should just wait around until the next time we send events, and then we'll send the next batch.

There's potentially an issue if there's a problem with one particular Tracks event that causes the batch to be rejected; that batch would remain as the first batch, and keep retrying indefinitely, preventing later events from being sent. But I don't know if that's even a possibility, and if it is, it's no worse than the situation we're already in — a single invalid event would already cause all the other events to fail, even without that change. So I'm not concerned about fixing that in this PR.

[bjhomer]

Have you considered moving the call to a dedicated queue?
Just curious if you found issues with changing queue or if it's something that wasn't considered in the interest of getting the fix out.

Yeah, I think I would like to move it to another queue, but I was going for a smaller change in the interest of getting a fix out sooner.

[bjhomer]

And really, our events are already running on a background context… the problem is that because the main thread still needs to access that context (via -performAndWait or -perform), the main thread can be blocked if the context is busy doing something else on another thread.

A better solution would probably want to have two separate contexts; one for writing, and one for reading. But that seems quite a bit more complex than what I have time to allocate to this at the moment. :)