Bug 241071 - Patch to speed up sos processing
Patch to speed up sos processing
Status: CLOSED CURRENTRELEASE
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: sos (Show other bugs)
5.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Adam Stokes
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-05-23 17:07 EDT by Karl Abbott
Modified: 2009-06-08 13:23 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-08 13:23:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Patch for helpers.py that improves the performance of sos (1.83 KB, patch)
2007-05-23 17:07 EDT, Karl Abbott
no flags Details | Diff

  None (edit)
Description Karl Abbott 2007-05-23 17:07:19 EDT
Description of problem:


Version-Release number of selected component (if applicable): sos-1.3-1.el5.noarch


Description:
The code in helpers.py to actually execute a command comes from the Python
Cookbook as stated in the code. As far as I can tell, that code is simply
executing the command and grabbing stdout and stderr. My patch removes the
Python Cookbook code and uses Python's commands library instead. The only
downside to this is that stderr and stdout are now combined into one chunk of
text -- so if you wanted to treat them separately, this wouldn't help. If it
doesn't matter that stderr and stdout are combined, then this works great.

I have done some rudimentary benchmarking with this. With the code that
currently exists in helper.py in sos, running an sos took 6m48s (Note there will
be error as I had to type my name and ticket number). Rebooting the machine and
running a patched version of sos took 3m27s. If we assume an error of 30 seconds
on either end for me to type in my name and ticket number, we still show a
significant speed increase.


Steps to Reproduce:
1. Run time sosreport -av
2. Reboot
3. Patch /usr/lib/python-2.4/site-packages/sos/helper.py
4. Run time sosreport -av

Expected results:

Significant speed improvement with the patch.

Regards,
Karl J. Abbott
GSS
Comment 1 Karl Abbott 2007-05-23 17:07:19 EDT
Created attachment 155298 [details]
Patch for helpers.py that improves the performance of sos
Comment 2 Steve Conklin 2007-05-24 08:56:50 EDT
stdout and stderr are separated because stderr is put in the sos log, and stdout
goes into the report.

But -

It may be a good idea to combine them in the output to the report. Any errors
while running commands should be of interest to whoever is looking at the
report, and having errors buried in another log obscures them. I don't think
that there was any compelling reason that they were separated in the first place.
Comment 4 Navid Sheikhol-Eslami 2007-07-12 17:45:29 EDT
I agree, using popen unnecessarily complicates things. Committed to svn.

-- Navid
Comment 5 Karl Abbott 2007-09-13 15:39:45 EDT
This seems to be unpatched in the latest svn trunk:

https://hosted.fedoraproject.org/projects/sos/browser/trunk/src/lib/sos/helpers.py

Was this intended or was this accidental?
Comment 6 Navid Sheikhol-Eslami 2007-09-13 18:07:44 EDT
Hi Karl,

I initially applied the patch in revision 200:

  https://hosted.fedoraproject.org/projects/sos/changeset/200

I later found it that it created problems with SELinux, generating a lot of
warnings. I didn't have a chance to investigate this further, but for this
reason I decided to go back to using popen in a similar way it is used from
within commands.getstatusouput():

  https://hosted.fedoraproject.org/projects/sos/changeset/307#file1

Later on, I implemented a timeout to allow the main thread to kill the process
and continue (which is something we couldn't do with commands.getstatusoutput),
this is very helpful especially on systems where we might end up with processes
in D status:

  https://hosted.fedoraproject.org/projects/sos/changeset/390#file2

Honestly I haven't done any benchmarking, but as we are using a very similar
code as in getstatusoutput() (which also uses popen), the execution speed should
be fairly similar.

Have you had a chance to try the code in trunk ?

Thanks.

-- Navid

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