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

check_ntp_time and check_ntp return just first offset [sf#3552882] #1089

Closed
monitoring-user opened this issue Sep 24, 2013 · 1 comment
Closed

Comments

@monitoring-user
Copy link

Submitted by None on 2012-08-01 02:54:31

Both check_ntp_time and check_ntp return just first measured offset, instead of wanted average of four measurements. There is used bad index j (equeal to 0) instead of correct index i in avg_offset loop:

for(i=0; i<servers[best_index].num_responses;i++){
    avg_offset+=servers[best_index].offset[j];
}

So avg_offset counts num_responses times the same first measured offset, then it is divided by num_responses, and first measured offset is back in avg_offset, instead of wanted average.

Also, I think that counting just an arithmetic average is not very good thing. For example, when there are measured values 0.001, 1.000, 0.001 and 0.001, the result will be 0.25075, whereas I think that there should be some logic, which skips obviously bad value 1.000, and result should be 0.001.

waja added a commit to waja/monitoring-plugins that referenced this issue Oct 1, 2013
@ghost ghost assigned waja Jan 24, 2014
waja added a commit to waja/monitoring-plugins that referenced this issue Jan 24, 2014
1. A minor change which makes the AVG_NUM configurable by compiler define.
2. The more important change: A fix for a bug which caused the check_ntp_time
   and check_ntp_time check to always take the first response from ntp and not
   the avarage since "j" is not increased anywhere. "i" should be used there
   instead.

(Closes: monitoring-plugins#956)
(Closes: monitoring-plugins#1089)
(Closes: monitoring-plugins#1166)
waja added a commit to waja/monitoring-plugins that referenced this issue Jan 24, 2014
The more important change: A fix for a bug which caused the check_ntp_time
and check_ntp_time check to always take the first response from ntp and not
the avarage since "j" is not increased anywhere. "i" should be used there
instead.

Partly Closes monitoring-plugins#956 and Closes monitoring-plugins#1166
Closes: monitoring-plugins#1089
@waja
Copy link
Member

waja commented Jan 24, 2014

fixed with ccecba3

@waja waja closed this as completed Jan 24, 2014
@waja waja removed their assignment Oct 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants