This reverts commit d700c382791b6352bb80a0dc455589085881669f.
This commit is causing a timeout/lock wait condition when using the in
memory rpc bus. It exposed in the Nova unit / functional tests which use
this extensively.
Change-Id: I9610a5533383955f926dbbb78ab679f45cd7bcdb
Closes-Bug: #1514876
This change formalises locking in MessageHandlingServer. It allows the
user to make calls in any order and it will ensure, with locking, that
these will be reordered appropriately. It also adds locking for
internal state when using the blocking executor, which closes a number
of races.
It fixes a regression introduced in change
gI3cfbe1bf02d451e379b1dcc23dacb0139c03be76. If multiple threads called
wait() simultaneously, only 1 of them would wait and the others would
return immediately, despite message handling not having completed.
With this change only 1 will call the underlying wait, but all will
wait on its completion.
We add a common logging mechanism when waiting too long. Specifically,
we now log a single message when waiting on any lock for longer than
30 seconds.
We remove DummyCondition as it no longer has any users.
Change-Id: I9d516b208446963dcd80b75e2d5a2cecb1187efa
ListenerSetupMixin.ThreadTracker was reading self._received_msgs
unlocked and sleep/looping until the desired value was reached.
Replaced this pattern with a threading.Condition.
Change-Id: Id4731caee2104bdb231e78e7b460905a0aaf84bf
This fixes a race due to the quirkiness of the blocking executor. The
blocking executor does not create a separate thread, but is instead
explicitly executed in the calling thread. Other threads will,
however, continue to interact with it.
In the non-blocking case, the executor will have done certain
initialisation in start() before starting a worker thread and
returning control to the caller. That is, the caller can be sure that
this initialisation has occurred when control is returned. However, in
the blocking case, control is never returned. We currently work round
this by setting self._running to True before executing executor.start,
and by not doing any locking whatsoever in MessageHandlingServer.
However, this current means there is a race whereby executor.stop()
can run before executor.start(). This is fragile and extremely
difficult to reason about robustly, if not currently broken.
The solution is to split the initialisation from the execution in the
blocking case. executor.start() is no longer a blocking operation for
the blocking executor. As for the non-blocking case, executor.start()
returns as soon as initialisation is complete, indicating that it is
safe to subsequently call stop(). Actual execution is done explicitly
via the new execute() method, which blocks.
In doing this, we also make FakeBlockingThread a more complete
implementation of threading.Thread. This fixes a related issue in
that, previously, calling server.wait() on a blocking executor from
another thread would not wait for the completion of the executor. This
has a knock-on effect in test_server's ServerSetupMixin. This mixin
created an endpoint with a stop method which called server.stop().
However, as this is executed by the executor, and also joins the
executor thread, which is now blocking, this results in a deadlock. I
am satisfied that, in general, this is not a sane thing to do.
However, it is useful for these tests. We fix the tests by making the
stop method non-blocking, and do the actual stop and wait calls from
the main thread.
Change-Id: I0d332f74c06c22b44179319432153e15b69f2f45
test_server_wait_method was calling server.wait without having
previously called server.start and server.stop. This happened to work
because it also injected server._executor_obj. This is problematic,
though, as it assumes internal details of the server and does not
represent the calling contract of server.wait, which is that it must
follow server.stop (which must itself also follow server.start).
This change makes the necessary changes to call server.wait in the
correct sequence.
Change-Id: I205683ac6e0f2d64606bb06d08d3d1419f7645f4
MessageHandlingServer has both MessageHandlingServer.executor, which
is the name of an executor type, and MessageHandlingServer._executor,
which is an instance of that type. Ideally we would rename
MessageHandlingServer.executor, but as this is referenced from outside
the class we change _executor instead to _executor_obj.
Change-Id: Id69ba7a0729cc66d266327dac2fd4eab50f2814c
Instead of having to spin in the wait method, just use
a condition and block until stopping has actually happened,
when stop happens, it will use the notify_all method to let
any blockers release.
Closes-Bug: #1505730
Change-Id: I3cfbe1bf02d451e379b1dcc23dacb0139c03be76
Previously, the AMQP 1.0 driver did not pass the proper hostname to
pyngus. This prevented GSSAPI authentication from working properly.
Change-Id: Ibc6678e7cbae6dd5108d1650dbb8ddf837aa3648
Closes-Bug: #1503258
In this patch conf object passing through the fixtures to
objects is added
Also conf.prog and conf.project attributes are initialized
for tests
Change-Id: I4094043c3ea61cfd37cc998f140d506d4136e681
Partial-Implements: bp rabbit-pika
Previous driver and other locations in the code we used
zmq_async.import_zmq() and importutils.try_import() to
not fail fast when pyzmq and redis python packages are
not installed. If we don't do this, we are going to break
Nova/Cinder/Glance etc.
Closes-Bug: #1503499
Change-Id: I1529241b5e1d902b37d6b610646a5a46a18f13b0
Fixed and restored previously skipped test CallTestCase.test_timeout
_connect_to_host() and get_single_hosts() now accept optional timeout
parameter.
_connect_to_host() accepts optional retry parameter
If timeout value (in seconds) is greater than 0, the connection attempts
will be retried $retry times with a specified timeout.
Change-Id: I1c897d917d0defda581b8ff62e7a67e32f011fee
Closes-Bug: #1499713
We don't see it right now in logstash as we fail with
a Segmentation fault (core dumped) because of other
issues. But will end up seeing it once that is fixed.
Closes-Bug: #1492505
Change-Id: I6507e693fc929e03884f933bbda241f744d3a7c0
Minor syntax fixes to allow the AMQP 1.0 driver to run under Python
2.7 and 3.4.
Closes-Bug: #1477124
Change-Id: I8512bb8274fad78285ab59797ba9b3f0f8b5c171
This restores behavior as it was back in the Juno cycle. This was
changed in a refactor during the Kilo cycle (973301aa) when the kombu
failover_strategy was set to shuffle. But the failover_strategy only
affects failovers; it does not influence which host receives the
initial connection.
Thus, if multiple hosts are given, the first host will receive all of
the initial connections. By shuffling the hosts before building the
URL given to kombu, rudimentary load balancing is achieved and
connections are more evenly spread across broker nodes.
Change-Id: Ieecaf5f286f49302db29df9854359ef5d36b7a89
Remove unnecessary rpc_zmq_port option from zmq driver, because zmq
driver doesn't use static port any more - dynamic port binding is used
for running servers.
Closes-Bug: #1497277
Change-Id: I91e978347dd364b2d4c00bc223b0e3ecface7c43