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:
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*/
When I mentioned #if..#endif, I didn't mean it as a dogmatic
"conditional compilation is wrong" sense.  It was more about "the
high-level flow of logic should not have to know too much about
platform peculiarities".  As platform_capable_non_blocking_IO()
function would be a constant function after you compile it for a
single platform, if you add 10 instances of such if/else in a patch
that adds 250 lines, unless the change is to add a set of lowest
level helpers to be called from the higher-level flow of logic so
that the callers do not have to know about the platform details,
that's just as bad as adding 10 instances of #if..#endif.
quoted
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)
When (1) "parallelism with sequential output" is the desired
outcome, (2) on some platforms we haven't found a way to achieve
both, and (3) a non-sequential output is unacceptable, then
parallelism has to give :-(.

I was getting an impression from your "not buffer" suggestion that
"sequential output" would be the one that can be sacrificed, but
that is OK.  Until we find a way to achieve both at the same time,
achieving only either one or the other is better than achieving
nothing.
quoted
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 1:19 PM, Junio C Hamano [off-list ref] wrote:
Stefan Beller [off-list ref] writes:
quoted
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*/
When I mentioned #if..#endif, I didn't mean it as a dogmatic
"conditional compilation is wrong" sense.  It was more about "the
high-level flow of logic should not have to know too much about
platform peculiarities".  As platform_capable_non_blocking_IO()
function would be a constant function after you compile it for a
single platform, if you add 10 instances of such if/else in a patch
that adds 250 lines, unless the change is to add a set of lowest
level helpers to be called from the higher-level flow of logic so
that the callers do not have to know about the platform details,
that's just as bad as adding 10 instances of #if..#endif.
Right. Yeah I was just outlining the program flow as I understood it.

Specially:
 * It's fine if the platform doesn't support non blocking IO
 * but if the platform claims to support it and fails to, this is a hard error.
    How is this worse than the platform claiming to not support it at all?

I mean another solution there would be to try to set the non blocking IO
and if that fails we put that process-to-be-started into a special queue,
which will start the process once the stderr channel is not occupied any more
(i.e. the process which is the current output owner has ended or there is
no such process)

That way we would fall back to no parallelism in Windows and could
even gracefully fallback in other systems, which suddenly have problems
setting non blocking IO. (Though I suspect that isn't required here).
quoted
quoted
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)
When (1) "parallelism with sequential output" is the desired
outcome, (2) on some platforms we haven't found a way to achieve
both, and (3) a non-sequential output is unacceptable, then
parallelism has to give :-(.
Ok, then I will go that route.
I was getting an impression from your "not buffer" suggestion that
"sequential output" would be the one that can be sacrificed, but
that is OK.  Until we find a way to achieve both at the same time,
achieving only either one or the other is better than achieving
nothing.
I was not sure what is better to sacrifice. Scarifying parallelism sounds
safer to me (no apparent change compared to before).
quoted
quoted
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