Bug 1128715

Summary: [scale] unclosed files refernces hit the performance
Product: Red Hat Enterprise Virtualization Manager Reporter: Eldad Marciano <emarcian>
Component: vdsmAssignee: Oved Ourfali <oourfali>
Status: CLOSED CURRENTRELEASE QA Contact: Eldad Marciano <emarcian>
Severity: low Docs Contact:
Priority: unspecified    
Version: 3.4.1-1CC: bazulay, danken, gklein, lpeer, oourfali, ybronhei, yeylon, ykaul
Target Milestone: ovirt-3.6.0-rc   
Target Release: 3.6.0   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-02-03 14:01:14 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Infra RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Eldad Marciano 2014-08-11 11:59:03 UTC 
Description of problem:
there is many python modules running 'open(file).read()' with no close statement
close files object\references after read will reduce the file handlers (actual lsof capacity which running) under scale setup.

adding '<some object file>.close()' code line after read action required
this is a mandatory enhancement.
small change with big impact.



Version-Release number of selected component (if applicable):
vdsm-4.14.11-1.el6ev.x86_64

How reproducible:
100%


Steps to Reproduce:
1.grep '.read()' /usr/share/vdsm/*


Actual results:
by quick review there is many read action with no close statement

Expected results:
close statement after each one of the read() actions. 

Additional info:
Comment 1 Yaniv Bronhaim 2014-08-11 15:24:54 UTC 
Yeala, can you check that out? 

From what I see we don't have such parts of unclosed files in vdsm. all the grep '.read()' lines that I see are under with statement or part of tests (which can be fixed, but not urgently).

Please verify that this is the case and close if not require a patch.

Thanks.
Comment 2 Yaniv Bronhaim 2014-08-11 15:27:18 UTC 
btw, I don't see how it relates to scale. can you elaborate when did you get high load of file descriptors ? what steps do you run to see that? I might miss something.. but as far as I checked, we don't have fd leak due to open files.
Comment 3 Yaniv Bronhaim 2014-08-11 15:30:27 UTC 
oh .. I missed some of the grep :) we do have such in sampling.py, storage folder,  supervdsmServer and more. please go ahead and put those places under with statement and call close directly
Comment 4 Eldad Marciano 2014-08-12 09:21:49 UTC 
Actually using try finally is more best practice.

e.g:

f = None # in order to avoid reference before assignment 
try:
f = open(file)
f.read()
return SOMETHING
expect Exception:
# do something
finally: # finally, close file reference if exist
if f;
f.close()
Comment 5 Dan Kenigsberg 2014-08-13 12:16:28 UTC 
Please note that we should not call close() explicitly, but use "with", many of those fixed by Dima's http://gerrit.ovirt.org/26776
Comment 6 Eldad Marciano 2014-08-20 14:31:27 UTC 
(In reply to Dan Kenigsberg from comment #5)
> Please note that we should not call close() explicitly, but use "with", many
> of those fixed by Dima's http://gerrit.ovirt.org/26776
+ 1
Comment 7 Yeela Kaplan 2014-09-09 15:08:46 UTC 
Yaniv, 
Working on it. need to verify the patch.
Comment 11 Gil Klein 2016-02-03 14:01:14 UTC 
This bug was fixed and is slated to be in the upcoming version. As we
are focusing our testing at this phase on severe bugs, this bug was
closed without going through its verification step. If you think this
bug should be verified by QE, please set its severity to high and move
it back to ON_QA