On Wed, May 25, 2016 at 12:18 PM, Martin Sivak <[email protected]> wrote: >> but it should >> not try to implement __del__ - we had lot of trouble with such code in vdsm. > > Del is perfectly fine if done properly. Python objects should really > clean after themselves. It is common to abstract the user from the low > level networking code. For example all the SocketServer or xmlrpclib > abstractions close the sockets automatically.
$ grep __del__ /usr/lib64/python2.7/SocketServer.py $ grep __del__ /usr/lib64/python2.7/xmlrpclib.py $ grep __del__ /usr/lib64/python2.7/SimpleXMLRPCServer.py > The same is true about > file objects (closed when garbage collected). This is implemented as C extension, not using __del__. > > Context manager can (and should if possible) be used to handle > exceptions properly and to allow better control, but the default > behavior should still close the resources during __del__ too. I disagree, __del__ cannot be implemented properly on python 2. It is possible to close stuff automatically only in a c extension. Trying to implement will lead to endless troubles. Nir > > Martin > > > On Mon, May 23, 2016 at 2:57 PM, Nir Soffer <[email protected]> wrote: >> On Mon, May 23, 2016 at 3:46 PM, Martin Sivak <[email protected]> wrote: >>>> Explicit close is required. >>>> >>>> You should ensure that all resources are released using try finally. >>> >>> Not according to the code Simone found. The close is called by the >>> __del__ method explicitly. And Python is no C, we should really behave >>> "nice" by default. >> >> No, the code you found is misguided attempt and should be deleted from vdsm. >> >> The jsonrpc client can provide a context manger to help you close it, >> but it should >> not try to implement __del__ - we had lot of trouble with such code in vdsm. >> >> You must close the client explicitly when you are done. >> >> Nir >> >>> >>> Martin >>> >>> On Mon, May 23, 2016 at 2:40 PM, Nir Soffer <[email protected]> wrote: >>>> On Mon, May 23, 2016 at 11:15 AM, Martin Sivak <[email protected]> wrote: >>>>> Hi, >>>>> >>>>>> I know that we reconnect several times during hosted engine process. Do >>>>>> we >>>>>> close client when it is not used anymore? >>>>> >>>>> No, we are not closing it according to Simone, shouldn't it be >>>>> released automatically? We are using Python after all.. explicit close >>>>> is not exactly common there. >>>> >>>> Explicit close is required. >>>> >>>> You should ensure that all resources are released using try finally. >>>> >>>> Nir >>>> >>>>> >>>>> Martin >>>>> >>>>> On Mon, May 23, 2016 at 8:55 AM, Piotr Kliczewski <[email protected]> >>>>> wrote: >>>>>> Sandro, >>>>>> >>>>>> I know that we reconnect several times during hosted engine process. Do >>>>>> we >>>>>> close client when it is not used anymore? >>>>>> >>>>>> Please provide lsof for the process and the log. >>>>>> >>>>>> Thanks, >>>>>> Piotr >>>>>> >>>>>> On Mon, May 23, 2016 at 8:42 AM, Sandro Bonazzola <[email protected]> >>>>>> wrote: >>>>>>> >>>>>>> MainThread::WARNING::2016-05-23 >>>>>>> 07:09:38,629::hosted_engine::480::ovirt_hosted_engine_ha.agent.hosted_engine.HostedEngine::(start_monitoring) >>>>>>> Unexpected error >>>>>>> Traceback (most recent call last): >>>>>>> File >>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/agent/hosted_engine.py", >>>>>>> line 444, in start_monitoring >>>>>>> self._initialize_vdsm() >>>>>>> File >>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/agent/hosted_engine.py", >>>>>>> line 635, in _initialize_vdsm >>>>>>> timeout=envconstants.VDSCLI_SSL_TIMEOUT >>>>>>> File >>>>>>> "/usr/lib/python2.7/site-packages/ovirt_hosted_engine_ha/lib/util.py", >>>>>>> line >>>>>>> 187, in connect_vdsm_json_rpc >>>>>>> requestQueue=requestQueue, >>>>>>> File "/usr/lib/python2.7/site-packages/vdsm/jsonrpcvdscli.py", line >>>>>>> 222, >>>>>>> in connect >>>>>>> responseQueue) >>>>>>> File "/usr/lib/python2.7/site-packages/vdsm/jsonrpcvdscli.py", line >>>>>>> 212, >>>>>>> in _create >>>>>>> lazy_start=False) >>>>>>> File "/usr/lib/python2.7/site-packages/yajsonrpc/stompreactor.py", >>>>>>> line >>>>>>> 576, in StandAloneRpcClient >>>>>>> reactor = Reactor() >>>>>>> File "/usr/lib/python2.7/site-packages/yajsonrpc/betterAsyncore.py", >>>>>>> line 200, in __init__ >>>>>>> self._wakeupEvent = AsyncoreEvent(self._map) >>>>>>> File "/usr/lib/python2.7/site-packages/yajsonrpc/betterAsyncore.py", >>>>>>> line 164, in __init__ >>>>>>> map=map >>>>>>> File "/usr/lib64/python2.7/asyncore.py", line 650, in __init__ >>>>>>> self.set_file(fd) >>>>>>> File "/usr/lib64/python2.7/asyncore.py", line 657, in set_file >>>>>>> self.socket = file_wrapper(fd) >>>>>>> File "/usr/lib64/python2.7/asyncore.py", line 616, in __init__ >>>>>>> self.fd = os.dup(fd) >>>>>>> OSError: [Errno 24] Too many open files >>>>>>> >>>>>>> Simone, Rafael, Piotr, Martin, can you please investigate? >>>>>>> >>>>>>> vdsm-yajsonrpc-4.18.0-16.git51df339.el7.centos.noarch >>>>>>> >>>>>>> ovirt-hosted-engine-ha-2.0.0-0.2.master.20160520143206.20160520143149.gita012f18.el7.noarch >>>>>>> >>>>>>> -- >>>>>>> Sandro Bonazzola >>>>>>> Better technology. Faster innovation. Powered by community >>>>>>> collaboration. >>>>>>> See how it works at redhat.com >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> Devel mailing list >>>>> [email protected] >>>>> http://lists.ovirt.org/mailman/listinfo/devel _______________________________________________ Devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/devel
