Use timeout in connect function,

[dvoryanchikov]

avoiding sigpipe in case one of remote monitor systems is unreachable that causing loss all other communications, maybe later should make it optional config parameter.

Example.

Monitoring 3 upses in upsmon.conf:

MONITOR [email protected] 0 foo bar slave
MONITOR myups@localhost 0 user pass master
MONITOR snmp@localhost 0 user pass slave

MINSUPPLIES 0
SHUTDOWNCMD "/bin/true"
POLLFREQ 5
POLLFREQALERT 5
HOSTSYNC 15
DEADTIME 15
POWERDOWNFLAG /etc/killpower

NOTIFYMSG ONLINE "Electricity OK at %s"
NOTIFYMSG ONBATT "Electricity Problem at %s"
NOTIFYMSG COMMBAD "Electricity Unknown at %s"
NOTIFYMSG COMMOK "Electricity Recovery at %s"
NOTIFYFLAG ONLINE SYSLOG+EXEC
NOTIFYFLAG ONBATT SYSLOG+EXEC
NOTIFYFLAG COMMBAD SYSLOG+EXEC
NOTIFYFLAG COMMOK SYSLOG+EXEC

RBWARNTIME 43200
NOCOMMWARNTIME 3600
FINALDELAY 5

Before patch (waiting for default kernel timeouts and tcp_syn_retries, blocking all work for 190 seconds and finally got COMMBAD for all upses):

  55.088474 pollups: [email protected]
  55.088526 get_var: [email protected] / status
  60.088024 Poll UPS [[email protected]] failed - Server disconnected
  60.088067 do_notify: ntype 0x0005 (COMMBAD)
  60.088084 Electricity Unknown at [email protected]
  60.088339 Dropping connection to UPS [[email protected]]
  60.088451 pollups: myups@localhost
  60.088488 get_var: myups@localhost / status
  60.088740 parse_status: [OL]
  60.088762 parsing: [OL]
  60.088775 pollups: snmp@localhost
  60.088787 get_var: snmp@localhost / status
  60.088897 parse_status: [OL]
  60.088918 parsing: [OL]
  60.089287 Current power value: 0
  60.089317 Minimum power value: 0
  65.089355 Trying to connect to UPS [[email protected]]
 254.088019 UPS [[email protected]]: connect failed: Connection failure: Connection timed out
 254.088069 pollups: myups@localhost
 254.088089 get_var: myups@localhost / status
 254.088190 SIGPIPE: dazed and confused, but continuing...
 254.088235 Poll UPS [myups@localhost] failed - Write error: Broken pipe
 254.088252 do_notify: ntype 0x0005 (COMMBAD)
 254.088265 Electricity Unknown at myups@localhost
 254.088476 Dropping connection to UPS [myups@localhost]
 254.088524 pollups: snmp@localhost
 254.088542 get_var: snmp@localhost / status
 254.088608 SIGPIPE: dazed and confused, but continuing...
 254.088656 Poll UPS [snmp@localhost] failed - Write error: Broken pipe
 254.088673 do_notify: ntype 0x0005 (COMMBAD)
 254.088688 Electricity Unknown at snmp@localhost
 254.089331 Dropping connection to UPS [snmp@localhost]
 254.091799 Current power value: 0
 254.091918 Minimum power value: 0
 259.092149 Trying to connect to UPS [[email protected]]

After patch (Pause 10 seconds on unreachable entry, other connections still alive):

  20.045464 pollups: [email protected]
  20.045523 get_var: [email protected] / status
  25.042222 Poll UPS [[email protected]] failed - Server disconnected
  25.042285 do_notify: ntype 0x0005 (COMMBAD)
  25.042302 Electricity Unknown at [email protected]
  25.042513 Dropping connection to UPS [[email protected]]
  25.042572 pollups: myups@localhost
  25.042650 get_var: myups@localhost / status
  25.042869 parse_status: [OL]
  25.042908 parsing: [OL]
  25.042972 pollups: snmp@localhost
  25.043004 get_var: snmp@localhost / status
  25.043139 parse_status: [OL]
  25.043155 parsing: [OL]
  25.043641 Current power value: 0
  25.043656 Minimum power value: 0
  30.043714 Trying to connect to UPS [[email protected]]
  40.046171 UPS [[email protected]]: connect failed: Unknown error
  40.046229 pollups: myups@localhost
  40.046254 get_var: myups@localhost / status
  40.046473 parse_status: [OL]
  40.046490 parsing: [OL]
  40.046508 pollups: snmp@localhost
  40.046524 get_var: snmp@localhost / status
  40.046641 parse_status: [OL]
  40.046657 parsing: [OL]
  40.046948 Current power value: 0
  40.046961 Minimum power value: 0
  45.047023 Trying to connect to UPS [[email protected]]
  55.046134 UPS [[email protected]]: connect failed: Unknown error
  55.046176 pollups: myups@localhost
  55.046202 get_var: myups@localhost / status
  55.046353 parse_status: [OL]
  55.046371 parsing: [OL]

So, if timeout is defined since 587d5f8, why not to use it?!
Patch tested and working.

[clepple]

Hmm, this exposes another problem: when using the timeout patch, it is not obvious that a timeout has occurred. At the very least, this feature should fix the "connect failed: Unknown error" message.

I'm not especially happy with the connect timeout code anyway - it was added to support nut-scanner. But in the long run, if we are changing effective TCP timeout behavior, that timeout should be configurable.

[aquette]

trying to resurrect the discussion:

• little progress was made on the general network timeout consideration with the instcmd/setvar status tracking (INSTCMD and SET VAR status tracking implementation  #659)
• upscli_connect() should remain blocking as before
• however, upsmon could consider using upscli_tryconnect(), with either an upsmon.conf specified timeout, or otherwise our default network timeout (https://github.com/networkupstools/nut/blob/master/include/common.h#L57)

@clepple @zykh @jimklimov thoughts and comments?

[jimklimov]

Just to be sure, this is orthogonal to rapid data stale-unstale sequences reported a few weeks ago (as #651 OTOH)?

[jimklimov]

Looking at this PR open for so long (and with useful discussion above), I think this particular change will not be merged due to reasons articulated above, but a similar feature (with configuration, docs and all) can be considered in a future release.