use timeout option when timeout config option is set #101

sa-linetco · 2019-08-22T10:44:46Z

fixes #100

mondwan · 2019-08-22T12:19:41Z

Thanks for your patch. But, that's in purpose to use -w instead of -W. Let me know if I am wrong. According to my experience, the option you suggest cannot guarantee the command exits within that period. Usually, I don't care how many pings have been sent. But, I do care there is a response (no matter success or not) within a certain time frame. That's why I use the deadline option. It returns immediately after getting first successful response. Otherwise, it is guaranteed to be closed for seconds I defined. 在 2019年8月22日週四 18:44，sa-linetco <[email protected]> 寫道：

…

fixes #100 <#100> ------------------------------ You can view, comment on, or merge this pull request online at: #101 Commit Summary - use timeout option when timeout config option is set File Changes - *M* lib/builder/linux.js <https://github.com/danielzzz/node-ping/pull/101/files#diff-0> (2) Patch Links: - https://github.com/danielzzz/node-ping/pull/101.patch - https://github.com/danielzzz/node-ping/pull/101.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#101?email_source=notifications&email_token=AA5YBI7ZFMSQCC4XGMGTIKLQFZUZ7A5CNFSM4IOTRXE2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HGX6OZQ>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5YBI3LBORB5ONK4SIJ53DQFZUZ7ANCNFSM4IOTRXEQ> .

sa-linetco · 2019-08-22T12:35:25Z

With the timeout option set, ping also immediately returns on the first successful ping.

time /bin/ping -n -w 5 -c 1 192.168.133.33
PING 192.168.133.33 (192.168.133.33) 56(84) bytes of data.
64 bytes from 192.168.133.33: icmp_seq=1 ttl=64 time=2.35 ms

--- 192.168.133.33 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 2.358/2.358/2.358/0.000 ms

real	0m0,007s
user	0m0,001s
sys	0m0,004s

time /bin/ping -n -W 5 -c 1 192.168.133.33
PING 192.168.133.33 (192.168.133.33) 56(84) bytes of data.
64 bytes from 192.168.133.33: icmp_seq=1 ttl=64 time=5.11 ms

--- 192.168.133.33 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 5.110/5.110/5.110/0.000 ms

real	0m0,010s
user	0m0,001s
sys	0m0,004s

As described in #100 -W also guarantees that the ping process exits after the given timeout.

The difference between timeout and deadline is that deadline will ignore the count parameter in case the ping requests are not being answered. But as ping is called with -n (or -c) in the module, I expect that only the given number of ping requests is sent (which is also what I'd expect after reading the documentation: * @property {number} min_reply - Exit after sending number of ECHO_REQUEST.

sa-linetco · 2019-08-22T12:43:26Z

If you want to conserve the possibility to set the deadline option on the javascript side, I'd suggest adding a deadline config option to the ping module. Translating timeout to the deadline ping option seems counterintuitive to me (especially as there is a timeout option available).

mondwan · 2019-08-23T02:20:39Z

I have done some experiments regarding to the deadline, and timeout option.

In your scenario, one ping packet, both options guarantee the exit time of command ping. But, timeout cannot do so if number of ping packets are more than 1, and the host is not reachable.

The deadline one

time ping -n -w 10 -c 10 1.2.3.4
PING 1.2.3.4 (1.2.3.4) 56(84) bytes of data.

--- 1.2.3.4 ping statistics ---
10 packets transmitted, 0 received, 100% packet loss, time 9209ms


real	0m10.001s
user	0m0.002s
sys	0m0.000s

The timeout one

time ping -n -W 10 -c 10 1.2.3.4
PING 1.2.3.4 (1.2.3.4) 56(84) bytes of data.

--- 1.2.3.4 ping statistics ---
10 packets transmitted, 0 received, 100% packet loss, time 9218ms


real	0m19.222s
user	0m0.000s
sys	0m0.003s

For host is reachable case, the result sames as the unreachable case mentioned above.

To conclude, I agree with your changes on timeout option (-w to -W), but we do need a new deadline option, which is linux specific, and write it down in the README FAQ section for this subtle usage.

Would you mind to help me to implement them?

sa-linetco · 2019-08-23T07:23:27Z

Would you mind to help me to implement them?

Sure thing! As I do not have a mac available, could you please make sure that the mac version also inherits the unix ping options of -w for deadline and -W for timeout?
On the windows parameter builder I'd suggest to log a warning if deadline option is used.

mondwan · 2019-08-23T07:26:24Z

Thanks. For the MAC one, I will check that later on. For the window one, I agree with your solution. 在 2019年8月23日週五 15:23，sa-linetco <[email protected]> 寫道：

…

Would you mind to help me to implement them? Sure thing! As I do not have a mac available, could you please make sure that the mac version also inherits the unix ping options of -w for deadline and -W for timeout? On the windows parameter builder I'd suggest to log a warning if deadline option is used. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#101?email_source=notifications&email_token=AA5YBI2UOYRYRQ7SITXYNUTQF6F67A5CNFSM4IOTRXE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD47LSPQ#issuecomment-524204350>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5YBIY24LYNZPRSUXGKKN3QF6F67ANCNFSM4IOTRXEQ> .

sa-linetco · 2019-08-23T09:25:30Z

For now I implemented the same behaviour for mac as I did for windows. When the deadline option is set, an error is thrown.

sa-linetco · 2019-08-23T14:09:07Z

Another fact I found out while working on this:
-w on windows and -W on linux do not show the same behaviour.
I think we should change the documentation regarding the configuration object accordingly.

Example (windows, unreachable ip):

> Measure-Command {start-process ping -argumentlist "-w 7000 -n 3 172.29.14.32" -wait}
TotalSeconds      : 23,5091395

Example (windows, reachable ip)

> Measure-Command {start-process ping -argumentlist "-w 7000 -n 3 192.168.133.33" -wait}
TotalSeconds      : 3,0480599

Example (linux, unreachable ip):

:~$ time ping -W 7 -c 3 172.29.14.32
PING 172.29.14.32 (172.29.14.32) 56(84) bytes of data.

--- 172.29.14.32 ping statistics ---
3 packets transmitted, 0 received, 100% packet loss, time 2038ms

real	0m9,040s
user	0m0,001s
sys	0m0,000s

Example (linux, reachable ip)

time ping -W 7 -c 3 192.168.133.33
PING 192.168.133.33 (192.168.133.33) 56(84) bytes of data.
64 bytes from 192.168.133.33: icmp_seq=1 ttl=64 time=2.65 ms
64 bytes from 192.168.133.33: icmp_seq=2 ttl=64 time=3.65 ms
64 bytes from 192.168.133.33: icmp_seq=3 ttl=64 time=2.67 ms

--- 192.168.133.33 ping statistics ---
3 packets transmitted, 3 received, 0% packet loss, time 2004ms
rtt min/avg/max/mdev = 2.658/2.996/3.652/0.465 ms

real	0m2,012s
user	0m0,006s
sys	0m0,000s

mondwan · 2019-08-24T09:48:27Z

Thanks. These are great, and high quality patch.

Below are conclusion so far on this issue. Would you mind to implement the MAC OS side as described in below table?
For the window's ping behaviour, I cannot notices the differences you mentioned. Would you mind to elaborate the differences so that we can see how to update the document accordingly?

Conclusion

Type	MAC OS Ping	Linux Ping	Window ping
deadline	-t (unit sec)	-w (unit sec)	NA
timeout	-W (unit msec)	-W (unit sec)	-w (unit msec)

Experiments from MAC OS

Below are experiments on MAC OS

Not reachable cases

deadline option

MacBook-Air-4:~$ time ping -t 10 -c 15 1.2.3.4
PING 1.2.3.4 (1.2.3.4): 56 data bytes
Request timeout for icmp_seq 0
Request timeout for icmp_seq 1
Request timeout for icmp_seq 2
Request timeout for icmp_seq 3
Request timeout for icmp_seq 4
Request timeout for icmp_seq 5
Request timeout for icmp_seq 6
Request timeout for icmp_seq 7
Request timeout for icmp_seq 8

--- 1.2.3.4 ping statistics ---
10 packets transmitted, 0 packets received, 100.0% packet loss

real	0m10.008s
user	0m0.002s
sys	0m0.003s

timeout option

MacBook-Air-4:~$ time ping -W 1000 -c 15 1.2.3.4
PING 1.2.3.4 (1.2.3.4): 56 data bytes
Request timeout for icmp_seq 0
Request timeout for icmp_seq 1
Request timeout for icmp_seq 2
Request timeout for icmp_seq 3
Request timeout for icmp_seq 4
Request timeout for icmp_seq 5
Request timeout for icmp_seq 6
Request timeout for icmp_seq 7
Request timeout for icmp_seq 8
Request timeout for icmp_seq 9
Request timeout for icmp_seq 10
Request timeout for icmp_seq 11
Request timeout for icmp_seq 12
Request timeout for icmp_seq 13

--- 1.2.3.4 ping statistics ---
15 packets transmitted, 0 packets received, 100.0% packet loss

real	0m16.040s
user	0m0.002s
sys	0m0.005s

Reachable case

deadline

MacBook-Air-4:~$ time ping -t 1 -c 5 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=55 time=6.148 ms

--- 8.8.8.8 ping statistics ---
2 packets transmitted, 1 packets received, 50.0% packet loss
round-trip min/avg/max/stddev = 6.148/6.148/6.148/0.000 ms

real	0m1.007s
user	0m0.002s
sys	0m0.003s

Timeout

MacBook-Air-4:~$ time ping -W 1000 -c 5 8.8.8.8
PING 8.8.8.8 (8.8.8.8): 56 data bytes
64 bytes from 8.8.8.8: icmp_seq=0 ttl=55 time=5.667 ms
64 bytes from 8.8.8.8: icmp_seq=1 ttl=55 time=6.099 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=55 time=5.860 ms
64 bytes from 8.8.8.8: icmp_seq=3 ttl=55 time=14.086 ms
64 bytes from 8.8.8.8: icmp_seq=4 ttl=55 time=6.324 ms

--- 8.8.8.8 ping statistics ---
5 packets transmitted, 5 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 5.667/7.607/14.086/3.247 ms

real	0m4.022s
user	0m0.002s
sys	0m0.003s

sa-linetco · 2019-08-26T08:31:21Z

* Below are conclusion so far on this issue. Would you mind to implement the MAC OS side as described in below table?

Sure thing, I'll get on it later today.

sa-linetco · 2019-08-26T08:42:13Z

* For the window's ping behaviour, I cannot notices the differences you mentioned. Would you mind to elaborate the differences so that we can see how to update the document accordingly?

Timeout: 7
Count: 3
Time for the ping process to exit:

	unreachable IP	reachable IP
linux	9	2
windows	23	3

The documentation for the configuration objects states that timeout determines the time for the ping process to exit, which is not what timeout stands for.
See:

@Property {number} timeout - Time duration, in seconds, for ping command to exit

My suggestion for the timeout option:

 * @property {number} timeout - Timeout in seconds for each ping request. Behaviour varies between platforms. Check platform ping documentation for more information.

Address the discussions on `timeout` and `deadline` in the README

mondwan · 2019-08-27T15:06:35Z

Great. Thanks for your patch. I have link up the discussions here in the README.

If there are no problems at all, I will merge this MR in a day or 2

sa-linetco · 2019-09-16T07:33:39Z

Hi @mondwan , are there any problems that arose with this version which need addressing?

mondwan · 2019-09-16T07:39:50Z

Ah, I forgot to press the button. Thanks for the remind.

sa-linetco · 2019-09-17T07:39:22Z

Thanks for the merge! Do you have any info on when to expect a release on npm?

mondwan · 2019-09-17T07:43:33Z

No idea. You need to contact danielzzz directly since I don't have access on npm 在 2019年9月17日週二 15:39，sa-linetco <[email protected]> 寫道：

…

Thanks for the merge! Do you have any info on when to expect a release on npm? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#101?email_source=notifications&email_token=AA5YBI3TLFP4EZWPPRSH6PDQKCCS3A5CNFSM4IOTRXE2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD63TSSQ#issuecomment-532101450>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5YBIYT3VXTPHIEEQN2P3LQKCCS3ANCNFSM4IOTRXEQ> .

use timeout option when timeout config option is set

043b3da

add support for the deadline option on linux platform

4e60728

sa-linetco and others added 4 commits August 26, 2019 16:17

add support for the deadline option on mac platform

c0d6fd5

fix unit for mac timeout

ffcc052

correct windows configuration object documentation

5a4a33f

Update README.md

5be9f2c

Address the discussions on `timeout` and `deadline` in the README

mondwan merged commit 1592b68 into danielzzz:master Sep 16, 2019

mondwan mentioned this pull request Apr 13, 2020

about ping speed #117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use timeout option when timeout config option is set #101

use timeout option when timeout config option is set #101

sa-linetco commented Aug 22, 2019

mondwan commented Aug 22, 2019 via email

sa-linetco commented Aug 22, 2019 •

edited

Loading

sa-linetco commented Aug 22, 2019

mondwan commented Aug 23, 2019 •

edited

Loading

sa-linetco commented Aug 23, 2019 •

edited

Loading

mondwan commented Aug 23, 2019 via email

sa-linetco commented Aug 23, 2019

sa-linetco commented Aug 23, 2019

mondwan commented Aug 24, 2019 •

edited

Loading

sa-linetco commented Aug 26, 2019

sa-linetco commented Aug 26, 2019 •

edited

Loading

mondwan commented Aug 27, 2019 •

edited

Loading

sa-linetco commented Sep 16, 2019

mondwan commented Sep 16, 2019

sa-linetco commented Sep 17, 2019

mondwan commented Sep 17, 2019 via email

use timeout option when timeout config option is set #101

use timeout option when timeout config option is set #101

Conversation

sa-linetco commented Aug 22, 2019

mondwan commented Aug 22, 2019 via email

sa-linetco commented Aug 22, 2019 • edited Loading

sa-linetco commented Aug 22, 2019

mondwan commented Aug 23, 2019 • edited Loading

sa-linetco commented Aug 23, 2019 • edited Loading

mondwan commented Aug 23, 2019 via email

sa-linetco commented Aug 23, 2019

sa-linetco commented Aug 23, 2019

Example (windows, unreachable ip):

Example (windows, reachable ip)

Example (linux, unreachable ip):

Example (linux, reachable ip)

mondwan commented Aug 24, 2019 • edited Loading

Conclusion

Experiments from MAC OS

Not reachable cases

deadline option

timeout option

Reachable case

deadline

Timeout

sa-linetco commented Aug 26, 2019

sa-linetco commented Aug 26, 2019 • edited Loading

mondwan commented Aug 27, 2019 • edited Loading

sa-linetco commented Sep 16, 2019

mondwan commented Sep 16, 2019

sa-linetco commented Sep 17, 2019

mondwan commented Sep 17, 2019 via email

sa-linetco commented Aug 22, 2019 •

edited

Loading

mondwan commented Aug 23, 2019 •

edited

Loading

sa-linetco commented Aug 23, 2019 •

edited

Loading

mondwan commented Aug 24, 2019 •

edited

Loading

sa-linetco commented Aug 26, 2019 •

edited

Loading

mondwan commented Aug 27, 2019 •

edited

Loading