----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16577/#review31355 -----------------------------------------------------------
Ship it! Looks fine to me, and if the tests work that's good enough for me. I would like to get Rafi's take on this as he is the expert on the code. As for the tests, I don't think the fact that its not strictly tied to ha should delay a fix. At least there are tests for python and heartbeats. The equivalent tests for c++ have never been written, - Gordon Sim On Jan. 6, 2014, 8:46 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16577/ > ----------------------------------------------------------- > > (Updated Jan. 6, 2014, 8:46 p.m.) > > > Review request for qpid, Gordon Sim and Rafael Schloming. > > > Bugs: qpid-5428 > https://issues.apache.org/jira/browse/qpid-5428 > > > Repository: qpid > > > Description > ------- > > QPID-5428: Heartbeats not in use when attempting to connect with python > client. > > Heartbeats ignored when opening a connection, could hang indefinitely > Need to cover 3 cases (test included): > - Connect sucessful but then broker stalls. > - Connect to a stalled broker that never responds. > - Fail-over to a stalled broker that never responds > > All cases are handled by the following fixes to driver.py: > - Check for heartbeats even before engine._connected since we may time out > before receiving open-ok if the peer is stalled and never sends data. > - Set _last_in and _last_out so that we time heartbeats from the start of the > connection if no data is ever sent or received. > - Call self.update_status in Driver.timeout to detect connection closed due to > heartbeat timeout (rather than a readable or writeable event.) > Make update_status a no-op if engine or transport are not yet set up. > - Don't consider reconnect complete in connect(), wait till we get the > open-ok. > See the comment on Driver._check_retry_ok() > > > Diffs > ----- > > /trunk/qpid/cpp/src/tests/ha_tests.py 1555989 > /trunk/qpid/python/qpid/messaging/driver.py 1555989 > > Diff: https://reviews.apache.org/r/16577/diff/ > > > Testing > ------- > > > Thanks, > > Alan Conway > >
