Add Timeout of 5m to zbx-speedtest.sh

[elekgeek]

This change is originally proposed by someone else @garbled1 but it was on the wrong file but still I believe this change should be applied to all scripts, nevertheless to speed things up, I just proposed the same exact change to avoid forcing people looking up why this script hangs and no longer works.

[pschmitt]

Thanks for your work on this.

As stated in my review of the original PR:

• please don't use full paths to binaries
• run generate-debian-files.sh and commit the updated Debian/ubuntu script
• the timeout should be user-configurable. I'm fine with a default of 5m but there's gotta people who will need to adjust that.
• also, the readme needs be adjusted accordingly

[elekgeek]

sure, let me read your script and see how I will do that correctly concerning the timeout. I assume also zabbix template has to be changed as well, am I correct?

[pschmitt]

Yeah probably, ideally the timeout would be a macro à la {#SPEEDTEST_TIMEOUT}. I forgot about that part.

[pschmitt]

Quick note: I just checked on a vanilla install of OpenWRT 19.03 -> timeout isn't installed by default.
This needs to be mentioned in the dependencies section of the README.

Sorry for dumping all these change requests on you like this, I'm really not trying to be a dick. Thanks for your interest in this particular piece of software and your work on this PR 🙏

[elekgeek]

I noticed that timeout 5m doesn't solve the hanging issue, I checked when I arrived home ps aux | grep zbx-speedtest.sh has been running for the last 2 hours.

I should find the actual reason behind this failure first.

[elekgeek]

1. I found that the user should be zabbix rather than zabbix-agent in zabbix-speedtest.service.

2. I don't believe that adding {$SPEEDTEST_TIMEOUT} is necessary to zabbix template template_speedtest.xml and zbx-speedtest.sh. I ran time speedtest --accept-license --accept-gdpr -f json for 50Mbps line 10 times, the average was 26.5 seconds, so having timeout 1m in zbx-speedtest.sh is more than enough to avoid speedtest troubles. Add to it is that the real value that matters is OnCalendar=*:0/x in zabbix-speedtest.timer, this value should never be less than 2 minutes, the same value of 2 minutes limitation applies to {$UPDATE_INTERVAL} that is mentioned in pt. 3, and this has to be reflected in README.

3. The more important thing was I added {$UPDATE_INTERVAL} macro to zabbix template, and replaced 5m value for Update interval with {$UPDATE_INTERVAL} so it is only once place where you have to change update interval for all items based on the value of OnCalender in the zabbix-speedtest.timer

If you agree on these changes let me know so I create complete pull request for all related files.

[pschmitt]

1. I found that the user should be zabbix rather than zabbix-agent in zabbix-speedtest.service.

This is distro-specific. To really make this work everywhere I guess I'll have to implement an install.sh script. Anyway: this is out of scope for this PR.

2. The real value that matters is OnCalendar=*:0/x

You are correct. This is definitely the better approach.

I ran time speedtest --accept-license --accept-gdpr -f json for 50Mbps line 10 times, the average was 26.5 seconds, so having timeout 1m in zbx-speedtest.sh is more than enough to avoid speedtest troubles

Not everybody has a 50Mb line... I won't merge a timeout of 1m. That's just too low IMHO.

3. The more important thing was I added {$UPDATE_INTERVAL} macro to zabbix template, and replaced 5m value for Update interval with {$UPDATE_INTERVAL} so it is only once place where you have to change update interval for all items based on the value of OnCalender in the zabbix-speedtest.timer

👍

[elekgeek]

I am new to this pull request thing to be honest, and it's making me more nervous when I say this is the first time I do a pull request :) anyway, just let me know what you need from me, and I will try to do the best I can.

[elekgeek]

man I re-read this discussion, this discussion should never be in a pull request :)

[pschmitt]

No harm done. Not all of it was off-topic ;)

[pschmitt]

@elekgeek you still on this? or should I discard this PR?

[elekgeek]

I did the mods locally, let me check what I did tomorrow, and will get back to you

…

On Sun, Nov 22, 2020, 12:43 AM Philipp Schmitt ***@***.***> wrote: @elekgeek <https://github.com/elekgeek> you still on this? or should I discard this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5THD62L2MWYAF7QETM32DSRA7BLANCNFSM4OIOLGOQ> .

[pschmitt]