Nir Soffer has posted comments on this change. Change subject: stomp: make sure that stomp connection is active before sending messages ......................................................................
Patch Set 3: (3 comments) https://gerrit.ovirt.org/#/c/49000/3//COMMIT_MSG Commit Message: Line 14: Line 15: Here is the flow how the issue happens: Line 16: 1. Client connects to the server Line 17: 2. Client sends CONNECT and SUBSCRIBE frames Line 18: 3. Client sends SEND fame fame -> frame Line 19: 4. Server receives all 3 frames and starts to process them Line 20: 5. Server attempts to send MESSAGE frame Line 21: 6. Server sends CONNECTED Line 22: https://gerrit.ovirt.org/#/c/49000/3/lib/yajsonrpc/stomp.py File lib/yajsonrpc/stomp.py: Line 542 Line 543 Line 544 Line 545 Line 546 Don't we need to clear self._connected when client is disconnected? Line 506: raise StompError(frame, frame.body) Line 507: Line 508: def send(self, destination, data="", headers=None): Line 509: if not self._connected.wait(timeout=CALL_TIMEOUT): Line 510: raise StompError("Timeout occured during connecting") This means that sending after client was disconnected will introduce unneeded delay instead of failing immediately. I think connection should be explicit. We should have a connect() method, waiting until a client is connected. Once a client is connected, send does not need any waits, and should fail quickly if a client was disconnected. Line 511: Line 512: final_headers = {"destination": destination} Line 513: if headers is not None: Line 514: final_headers.update(headers) -- To view, visit https://gerrit.ovirt.org/49000 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4f6881d7966fa47031d027740633ca83c834387f Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
