feat(event processor): change batching to be compliant with Event Processor #262
Conversation
jaeopt
commented
Sep 4, 2019
jaeopt
commented
- Add flush on {batchSize hit, revision change, project id change}
- Clean up batch (batch as much as possible instead of falling back to a single event mode when batch failure detected in an event group)
- Support maxQueueSize (configurable)
- Add many more unit tests to increase coverage
- Event notification is not included in this PR. It'll be prepared in a separate PR.
| unsubscribe() | ||
| } | ||
|
|
||
| func addProjectChangeNotificationObservers() { | ||
| NotificationCenter.default.addObserver(forName: .didReceiveProjectIdChange, object: nil, queue: nil) { (notif) in |
There was a problem hiding this comment.
Can we change these to didReceiveOptlyProjectIdChange? Or something with Optimizely in there?
| dataStore.save(item: event) | ||
|
|
||
| setTimer() | ||
| if dataStore.count == batchSize { |
There was a problem hiding this comment.
greater than and equal would probably be safer
|
|
||
| guard let batchEvent = batched else { | ||
| // discard events that create invalid batch and continue | ||
| removeStoredEvents(num: numEvents) |
There was a problem hiding this comment.
i'm not sure this makes sense. you are saying if batching failed remove all the events that were attempted to batch.
There was a problem hiding this comment.
In fact, it discards the single event when an invalid event is detected. The comment is misleading. What happens:
- if an invalid one is found while batching, it batches all the valid ones before the invalid one and send it out.
- when trying to batch next, it finds the invalid one at the header. It discards that specific invalid one and continue batching next ones.
I'll fix the comments to avoid confusion.
There was a problem hiding this comment.
Ok, I got it. But, numEvents should always be 1, right? Can you call that out somehow? You could have passed 1 into here to say remove the offending one. But, with numEvents, its hidden unless you look at the batch function and figure out where it passes back null. Guard numEvents == 1?
jaeopt
changed the title
