[ovirt-devel] FD leak while initializing vdsm

[Martin Sivak]

> 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. The same is true about
file objects (closed when garbage collected).

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.

Martin


On Mon, May 23, 2016 at 2:57 PM, Nir Soffer <nsoffer at redhat.com> wrote:
> On Mon, May 23, 2016 at 3:46 PM, Martin Sivak <msivak at redhat.com> 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 <nsoffer at redhat.com> wrote:
>>> On Mon, May 23, 2016 at 11:15 AM, Martin Sivak <msivak at redhat.com> 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 <pkliczew at redhat.com> 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 <sbonazzo at redhat.com>
>>>>> 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
>>>> Devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/devel