Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow JS tracker to change heart beat wait interval exponentially #8213

Closed
wants to merge 2 commits into from

Conversation

diosmosis
Copy link
Member

Adds two parameters to enableHeartBeatTimer() in the JS tracker: the delay multiplier and the maximum delay. After each ping, the delay is multiplied by the multiplier (so if 2 is used, the delay doubles), until it reaches or exceeds the maximum delay. Then that delay is used the whole time.

Useful to reduce load for websites/webapps where users view a single page for a long time. A website that streams video, is an example of such a website.

Refs #8069

CC @mattab

diosmosis added 2 commits June 25, 2015 05:14
@robocoder
Copy link
Contributor

I like that this is optional. Since 'ping' requests were intended to increase the accuracy of the visit length, an exponentially increasing interval conversely reduces the accuracy.

Instead of a multiplier and maximum delay, one of the things I had originally thought about was allowing the delay to be a callback function which returned the delay.

@diosmosis
Copy link
Member Author

Allowing a callback is something I thought about too, however I thought I had a couple of hours to finish the addition, so I threw it out :)

We could easily recognize the multiplier parameter as a function and if so treat it as a callback. Keeping a maximum timeout still seems like a good idea to me, we can't be sure the code used will always be correct, and if users want they can set a ridiculously high value if they want.

One potential issue is that neither of these options is particularly user friendly, especially to non-coders. I think in this case it's ok, because the potential users of this specific feature shouldn't be the normal user. Someone operating a video streaming service or a web app should have access to programmers (if they aren't programmers themselves), so they could manage and fine tune the feature. Other users that don't have a complicated website w/ high traffic, and only want to reduce load for, say, shared hosting, don't necessarily have to use the heart beat feature.

@tsteur
Copy link
Member

tsteur commented Jun 25, 2015

So maybe instead of having 3 options (or with callback 4) we could have one bool option $increasePingIntervalOverTime

  1. If disabled, which would be default, we ping every 15 seconds
  2. If enabled, we increase over time (maybe not exponentially and not too fast increase the interval)

One other option $callback that let's a user customize it.

I like API's that are simple to use and look simple in code (I mean simple to implement) but still gives you the power to adjust to the needs if one wants to which would be given with the callback. Otherwise we end up having 4 or 5 or 6 options at some point which nobody understands and is buggy.

@mattab
Copy link
Member

mattab commented Sep 11, 2015

Thanks for the PR, we will not merge it as is for now. I'm closing and we will discuss & later take a decision in #8049

@mattab mattab closed this Sep 11, 2015
@mattab mattab deleted the ping_interval_change branch December 3, 2015 23:02
@hpvd
Copy link

hpvd commented Dec 21, 2015

new try with some more explanation why something like this is really needed #9423

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants