matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
393 stars 164 forks source link

sdk - android - max Retry option #241

Open arul-prasad opened 5 years ago

arul-prasad commented 5 years ago

In the DefaultDispatcher, we are doing a re-try when the track call goes unsuccess and the events cache has elements. Possible to have MAX_RETRY option? by this way, we can control the app running in thread without re-trying always till app is closed or kill.

For example i expect when piwik (matomo) server was down, I expect app can re-try only 5 times.

https://github.com/matomo-org/matomo-sdk-android/blob/master/tracker/src/main/java/org/matomo/sdk/dispatcher/DefaultDispatcher.java

d4rken commented 5 years ago

What would happen after exceeding the MAX_RETRY value?

How would you differentiate server down from connectivity issues, where the user is, e.g. in a subway?

arul-prasad commented 5 years ago

My intention is to stop the thread keep on running when there is server down or connectivity issue, probably can we write the events cache in disk and try when app re-open to push the offline track?

d4rken commented 5 years ago

Did you notice this having a negative impact? The current code would have it retry every 5 minutes, a thread consumes virtually no resources while sleeping. If the app is in the background during that time the system will kill it anyways.

arul-prasad commented 5 years ago

agreed when we have retry every 5 minutes our impact is low and when the app is in background this will be killed. What happens when we have dispatchInterval 0 ?

mPiwikTracker = Piwik.getInstance(getReactApplicationContext()).newTracker(TrackerConfig.createDefault(url, id)); mPiwikTracker.setDispatchInterval(0);

in this case we will have sleepTime as 0. while (mRunning) { try { long sleepTime = mDispatchInterval; if (mRetryCounter > 1) sleepTime += Math.min(mRetryCounter * mDispatchInterval, 5 * mDispatchInterval); // Either we wait the interval or forceDispatch() granted us one free pass mSleepToken.tryAcquire(sleepTime, TimeUnit.MILLISECONDS); } catch (InterruptedException e) {Timber.tag(LOGGER_TAG).e(e); }

Note: I am trying to avoid queuing in the client app, and I am trying to dispatch the track immediately to the piwik server, so dispatchInterval is 0.

arul-prasad commented 5 years ago

I thought of adding below check in DefaultDispatcher, let me know your comments. especially when dispatchInterval is zero. (no wait)

    if (mRetryCounter > 1) {
        if (mDispatchInterval > 0) {
            sleepTime += Math.min(mRetryCounter * mDispatchInterval, 5 * mDispatchInterval);
        } else {
            sleepTime += Math.min(mRetryCounter * DEFAULT_DISPATCH_INTERVAL, 5 * DEFAULT_DISPATCH_INTERVAL);
        }
    }