Code review comment for lp:~termie/nova/rpc_multicall

Revision history for this message
Chris Behrens (cbehrens) wrote :

Jay: I think it may be so that there's less CPU spinning... though this is just in fakerabbit.

I'm worried about the 'time.sleep(0.01)' in MulticallWaiter, though. That's a lot of send/recvs spinning until the result actually comes into the queue...

What about this:

    def wait(self):
        while not self._closed:
            try:
                self._consumer.wait(limit=1)
            except Exception:
                self.close()
                raise

            result = self._results.get()
            if isinstance(result, Exception):
                self.close()
                raise result
            if result == None:
                self.close()
                raise StopIteration
            yield result
        raise StopIteration

That removes the need for the sleep and will be less harsh on rabbit.. wait() will block until a result is received (eventlet will be polling for data). And if one happens to call wait() after close(), StopIteration is raised.

Couple other things:

1) Service.kill puts self.conn back into the pool, but it never came out of the pool in the first place. I'd just remove the 'put'. No need to use the pool for it right now, and it somewhat breaks the pool if ConsumerSet needs to reconnect to rabbit.
2) Should this code that kills the csetthread, etc, actually be in Service.stop?
3) There's no handling of reconnecting to rabbit if the connection dies. We really need a larger re-factor. But since only 'call' is using this right now, and 'call' already didn't handle reconnecting, there's not really any behavior change.

« Back to merge proposal