search

snowplow / snowplow-java-tracker

Snowplow event tracker for Java. Add analytics to your Java desktop and server apps, servlets and games. (See also: snowplow-android-tracker)
http://snowplowanalytics.com
Apache License 2.0
24 stars 36 forks source link

Add retry to in-memory storage system (close #156) #305

Closed mscwilson closed 2 years ago

mscwilson commented 2 years ago

For issue #156.

This (breaking) change adds a backoff and retry mechanism to the Emitter. Previously, events that failed to send were returned, so that the user could choose to use a callback to track them again. Now, when a request fails, the events are returned to the event buffer for retry in a subsequent request.

When a batch of events (TrackerPayloads) is removed from the event buffer for sending, the events are now copied into a hashmap, and turned into a BatchPayload, a new wrapper class. BatchPayload stores the TrackerPayloads for the request along with the hashmap key. If the request is sent successfully, the batched events are deleted from the hashmap. If not, they are inserted back at the front of the event buffer. The aim is to send events approximately first in, first out.

The backoff mechanism uses the schedule() of ScheduledThreadPoolExecutor to delay all new requests after a failure has occurred. The initial delay is 50 ms, which increases exponentially with every subsequent failure. When one request succeeds, the retry delay returns to 0.

For testing purposes, it made sense here to allow users to set the InMemoryEventStore event buffer capacity on creation. The LinkedBlockingDeque by default has a capacity of Integer.MAX_VALUE.

Having a bufferCapacity field made the name bufferSize to mean batchSize extra confusing, so I changed it. It's not really part of retry though, so I might move these commits to their own issues. This name change is now issue #306.

While testing this, I discovered that processing Events into TrackerPayloads in the Tracker is very fast. This means that the Tracker threadpool I introduced in PR #293 isn't necessary. I've removed it (making PR #298 irrelevant).

Note to community contributor @AcidFlow: we merged in your PR #259 in version 0.11.0 which allowed users to select their own ExecutorService. My current changes now require the ScheduledExecutorService interface. Any thoughts on this change?

AcidFlow commented 2 years ago

Hey @mscwilson !

I haven't look at the whole changelog yet as I don't have much time, but regarding your change now requiring a ScheduledExecutorService I think this should be fine. IIRC my previous contribution was to give more control on the Executor service so you could control its thread factory, pool and rejected execution handler so users would control the amount of thread they use, and could add metrics on rejection for instance. Using a ScheduledExecutorService still gives the user the same control with more reliability thanks to your changes :)

mscwilson commented 2 years ago

Great, thanks @AcidFlow!