Nir Soffer has posted comments on this change. Change subject: xmlrpc: do not block forever when waiting on incoming messages ......................................................................
Patch Set 4: (3 comments) https://gerrit.ovirt.org/#/c/42915/4/lib/vdsm/xmlrpc.py File lib/vdsm/xmlrpc.py: Line 121 Line 122 Line 123 Line 124 Line 125 Also lets minimize the changes by not renaming here self.queue to self._requests. This rename is good but is not required for fixing the blocking issue. Line 123: Line 124: def __init__(self, RequestHandlerClass): Line 125: super(ConnectedTCPServer, self).__init__(None, RequestHandlerClass, Line 126: bind_and_activate=False) Line 127: self._lock = threading.RLock() We don't need the lock, TaskQueue is already thread safe. Line 128: self._requests = TaskQueue(sys.maxint) Line 129: Line 130: def add(self, connected_socket, socket_address): Line 131: with self._lock: Line 124: def __init__(self, RequestHandlerClass): Line 125: super(ConnectedTCPServer, self).__init__(None, RequestHandlerClass, Line 126: bind_and_activate=False) Line 127: self._lock = threading.RLock() Line 128: self._requests = TaskQueue(sys.maxint) Do we really want to allow 2**31 queued taks?! Currently we don't have a limit on the number of tasks because we were careless. Lets put a reasonable limit, signaling clients that they are sending too many requests. But this change is not related to the blocking issue, so better add a TODO in this patch and change the limit (and add handling for TooManyTasks) in another patch. Line 129: Line 130: def add(self, connected_socket, socket_address): Line 131: with self._lock: Line 132: self._requests.put((connected_socket, socket_address)) -- To view, visit https://gerrit.ovirt.org/42915 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I295e3099ea06d786741164e1f240f4662631bf8a Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Yaniv Bronhaim <ybron...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches