Nir Soffer has posted comments on this change. Change subject: json-rpc: Protocol detection ......................................................................
Patch Set 17: (1 comment) http://gerrit.ovirt.org/#/c/26300/17/vdsm/protocolDetector.py File vdsm/protocolDetector.py: Line 139: socket_address) Line 140: else: Line 141: self.log.warn("Unrecognized protocol from %s", Line 142: socket_address) Line 143: client_socket.close() > I am not really sure whether we are trying to do to much for the use case. We have more then 2 connections - in current xmlrpc code, we see typically 3 threads that are always responding to requests, and rarely a new thread is created. But engine has some code that close connections after one request - so some connections are going be closed, and new one will be created. I think that the current code will work most of the time, but it will also fail here and there. I think that more robust implementation is needed, and it is not hard to do as you can see in my example. Even more important than not closing good connection because data was not available, is not keeping too many connections pending. As soon as you took my advice to wait until the socket is ready, and put it in a dict, you also responsible to remove them from the dict, even if the client never send any data. So you would like a timeout anyway for this. Lets see what other think. Line 144: Line 145: def _create_socket(self, host, port): Line 146: addr = socket.getaddrinfo(host, port, socket.AF_INET, Line 147: socket.SOCK_STREAM) -- To view, visit http://gerrit.ovirt.org/26300 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id739a40e2b37dcc175137ec91cd5ec166ad24a75 Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
