Bug 1128715 - [scale] unclosed files refernces hit the performance
Summary: [scale] unclosed files refernces hit the performance
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: vdsm
Version: 3.4.1-1
Hardware: x86_64
OS: Linux
unspecified
low
Target Milestone: ovirt-3.6.0-rc
: 3.6.0
Assignee: Oved Ourfali
QA Contact: Eldad Marciano
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-08-11 11:59 UTC by Eldad Marciano
Modified: 2016-02-03 14:01 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-02-03 14:01:14 UTC
oVirt Team: Infra


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2016:0362 normal SHIPPED_LIVE vdsm 3.6.0 bug fix and enhancement update 2016-03-09 23:49:32 UTC
oVirt gerrit 31717 master MERGED use with statement to take care of unclosed files Never
oVirt gerrit 33859 master MERGED Change file() to open() continued Never

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


Note You need to log in before you can comment on or make changes to this bug.