interdiff hangs on massive patches

RESOLVED FIXED in Bugzilla 5.0

Status

()

RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

({regression})

Bugzilla 5.0
regression
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
our sysadmins have reported extremely long running processes which are performing interdiffs.

upon analysis, it looks like we're hitting ipc deadlocks with open3.

i suspect the right thing to do is to use IO:Select to read from the handles, however this means we'd have to read the whole interdiff output into a string and pass to patchreader instead of passing in a filehandle.

Comment 1

6 years ago
(In reply to Byron Jones ‹:glob› from comment #0)
> i suspect the right thing to do is to use IO:Select

Or back out bug 784352 entirely if it's responsible for too many regressions or performance regressions. That's the decision I would take if needed (before we reach 5.0rc1).
Depends on: 784352
Keywords: regression
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 5.0
Version: 4.2 → 4.5
(Assignee)

Comment 2

6 years ago
Created attachment 719387 [details] [diff] [review]
patch v1

for mod_cgi and mod_perl without perlio we need to use select.
mod_perl with perlio handles the select for us, so we can just slurp it.
Attachment #719387 - Flags: review?(LpSolit)

Comment 3

6 years ago
(In reply to Byron Jones ‹:glob› from comment #2)
> for mod_cgi and mod_perl without perlio we need to use select.
> mod_perl with perlio handles the select for us, so we can just slurp it.

Please add links pointing to some documentation to understand the rationale behind what you do in this patch. bug 784352 + this patch makes me think the code is overly complex for a very minor benefit.
(Assignee)

Comment 4

6 years ago
if the output from a pipe exceeds the buffer size, we need to loop and select each pipe to read all the response.  under mod_perl+perlio this won't work due to select being used internally.


http://perldoc.perl.org/IPC/Open3.html

"If you try to read from the child's stdout writer and their stderr writer, you'll have problems with blocking, which means you'll want to use select() or IO::Select"

http://perl.apache.org/docs/2.0/api/Apache2/SubProcess.html

"A pipe filehandle returned under perlio-disabled Perl needs to call select() if the other end is not fast enough to send the data, since the read is non-blocking.  A pipe filehandle returned under perlio-enabled Perl on the other hand does the select() internally, because it's really a filehandle opened via :APR layer"

> the code is overly complex for a very minor benefit.

this issue has been raised many times from bmo users; to the point where some simply refuse to use the interdiff function because they can't tell if they are seeing valid results.

Updated

5 years ago
Attachment #719387 - Flags: review?(LpSolit) → review?(dkl)
Comment on attachment 719387 [details] [diff] [review]
patch v1

Review of attachment 719387 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #719387 - Flags: review?(dkl) → review+

Updated

5 years ago
Flags: approval?

Updated

5 years ago
Flags: approval? → approval+
(Assignee)

Comment 6

5 years ago
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Attachment/PatchReader.pm
Committed revision 8862.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.