qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request 16577: QPID-5428: Heartbeats not in use when attempting to connect with python client.
Date Tue, 07 Jan 2014 22:40:02 GMT


> On Jan. 3, 2014, 12:22 a.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/tests/ha_tests.py, line 237
> > <https://reviews.apache.org/r/16577/diff/1/?file=413695#file413695line237>
> >
> >     This doesn't appear to be an HA test why is it in these tests?
> 
> Alan Conway wrote:
>     Agreed, should be in the qpid/python tests somewhere. I'll look for an appropriate
place.
> 
> Alan Conway wrote:
>     The trick here is that the failover part of the test requires starting and killing
a second broker, which is beyond the capabilities of the qpid/python tests.
>     Options I can see:
>     - leave it where it is since it tests failover which is HA-related
>     - create a new cpp/src/tests/python_client_test.py test suite (using brokertest.py)
with this test to start, can collect further python client tests there that need to start/stop
brokers but don't require the use of the HA plugin.
>     I will do the latter if you think it is worthwhile.
> 
> Andrew Stitcher wrote:
>     Hmm, I think the broker starting functionality in the python test framework is much
more recent than the original python tests, but perhaps the original python tests should actually
start the broker itself.
>     
>     On the other hand I think the python test can actually be used against both the c++
and java brokers, so perhaps it should only start the broker if how to do that can be specified.
>     
>     If this test is specific to the C++ broker then I think it probably should actually
start a new set of tests and we should migrate tests there that are specific to the C++ broker.
>     
>     I'll stop rambling now - hope that made sense.

See reply to comment below. The issue is not so much that this is specific to the C++ broker,
as that it needs brokers to be dynamically stopped and started during the test. That makes
it hard to use a static, externally-started broker the way the other tests under qpid/python
do.


- Alan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16577/#review31093
-----------------------------------------------------------


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message