Ensure queue is always < 1000 items. #751
Conversation
Previously we didn't cap the queue, and when we added the limit, we only removed the oldest element, rather than fully shrink the queue. Hence there are cases where the queue may already be larger than 1000 events. Now we delete as many events as required to trim the queue size. Ref: LIB-210
Codecov Report
@@ Coverage Diff @@
## master #751 +/- ##
==========================================
+ Coverage 86.67% 86.92% +0.25%
==========================================
Files 36 36
Lines 2581 2616 +35
Branches 177 176 -1
==========================================
+ Hits 2237 2274 +37
+ Misses 339 337 -2
Partials 5 5 |
|
We should update docs as well since we are dropping events. Why not have the logic try to flush the queue when it reaches 1000 and fallback to dropping events? |
|
The queue already flushes when events > flushCount. If we ever get to this state, it's typically because the device has been offline for an extended period and the last 1000-flushCount attempts have failed (by default flush count is 20, that means the last 9980 attempts have failed). I think it's ok to go ahead to removing the events rather than try to flush the queue yet again. Android also does the same. I'll add a note in the docs - we were already removing these messages before but looks like we didn't document the old behaviour. |
|
Docs updating here https://github.com/segmentio/site-docs/pull/2719 |
Previously we didn't cap the queue, and when we added the limit, we only removed the oldest element, rather than fully shrink the queue.
Hence there are cases where the queue may already be larger than 1000 events, and removing a single element at a time isn't as helpful. Now we delete as many events as required to trim the queue size.
Ref: LIB-210