Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:07:11

Stefan Beller [off-list ref] writes:
Another approach would be to test if we can set to non blocking and if
that is not possible, do not buffer it, but redirect the subcommand
directly to stderr of the calling process.

    if (set_nonblocking(pp->children[i].process.err) < 0) {
        pp->children[i].process.err = 2;
        degraded_parallelism = 1;
    }

and once we observe the degraded_parallelism flag, we can only
schedule a maximum of one job at a time, having direct output?
I would even say that on a platform that is _capable_ of setting fd
non-blocking, we should signal a grave error and die if an attempt
to do so fails, period.

On the other hand, on a platform that is known to be incapable
(e.g. lacks SETFL or NONBLOCK), we have two options.

1. If we can arrange to omit the intermediary buffer processing
   without butchering the flow of the main logic with many
   #ifdef..#endif, then that would make a lot of sense to do so, and
   running the processes in parallel with mixed output might be OK.
   It may not be very nice, but should be an acceptable compromise.

2. If we need to sprinkle conditional compilation all over the place
   to do so, then I do not think it is worth it.  Instead, we should
   keep a single code structure, and forbid setting numtasks to more
   than one, which would also remove the need for nonblock IO.

Either way, bringing "parallelism with sequential output" to
platforms without nonblock IO can be left for a later day, when we
find either (1) a good approach that does not require nonblock IO to
do this, or (2) a good approach to do a nonblock IO on these
platforms (we know about Windows, but there may be others; I dunno).

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

From: Stefan Beller <hidden>
Date: 2016-06-15 23:07:11

On Wed, Nov 4, 2015 at 12:42 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
Another approach would be to test if we can set to non blocking and if
that is not possible, do not buffer it, but redirect the subcommand
directly to stderr of the calling process.

    if (set_nonblocking(pp->children[i].process.err) < 0) {
        pp->children[i].process.err = 2;
        degraded_parallelism = 1;
    }

and once we observe the degraded_parallelism flag, we can only
schedule a maximum of one job at a time, having direct output?
I would even say that on a platform that is _capable_ of setting fd
non-blocking, we should signal a grave error and die if an attempt
to do so fails, period.
So more like:

    if (platform_capable_non_blocking_IO())
        set_nonblocking_or_die(&pp->children[i].process.err);
    else
        pp->children[i].process.err = 2; /* ugly intermixed output is possible*/
On the other hand, on a platform that is known to be incapable
(e.g. lacks SETFL or NONBLOCK), we have two options.

1. If we can arrange to omit the intermediary buffer processing
   without butchering the flow of the main logic with many
   #ifdef..#endif, then that would make a lot of sense to do so, and
   running the processes in parallel with mixed output might be OK.
   It may not be very nice, but should be an acceptable compromise.
From what I hear this kind of output is very annoying. (One of the
main complaints of repo users beside missing atomic fetch transactions)
2. If we need to sprinkle conditional compilation all over the place
   to do so, then I do not think it is worth it.  Instead, we should
   keep a single code structure, and forbid setting numtasks to more
   than one, which would also remove the need for nonblock IO.
So additional to the code above, we can add the
platform_capable_non_blocking_IO() condition to either the ramp up process,
or have a

    if (!platform_capable_non_blocking_IO())
        pp.max_processes = 1;

in the init phase. Then we have only 2 places that deal with the
problem, no #ifdefs
elsewhere.
Either way, bringing "parallelism with sequential output" to
platforms without nonblock IO can be left for a later day, when we
find either (1) a good approach that does not require nonblock IO to
do this, or (2) a good approach to do a nonblock IO on these
platforms (we know about Windows, but there may be others; I dunno).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help