845725 - interdiff hangs on massive patches

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[:glob ✱]

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

[Frédéric Buclin]

(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).

Comment 2

[:glob ✱]

Attachment: 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.

Comment 3

[Frédéric Buclin]

(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.

Comment 4

[:glob ✱]

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.

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 719387 [details] [diff] [review]
patch v1

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

r=dkl

Comment 6

[:glob ✱]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Attachment/PatchReader.pm
Committed revision 8862.