Re: [go-nuts] Re: Replacement for net.Error.Temporary in server Accept loops

2022-04-21 Thread Caleb Spare
On Thu, Apr 21, 2022 at 7:16 AM 'Bryan C. Mills' via golang-nuts
 wrote:
>
> Even ENFILE and EMFILE are not necessarily blindly retriable: if the process 
> has run out of files, it may be because they have leaked (for example, they 
> may be reachable from deadlocked goroutines).
> If that is the case, it is arguably better for the program to fail with a 
> useful error than to keep retrying without making progress.
>
> (I would argue that the retry loop in net/http.Server is a mistake, and 
> should be replaced with a user-configurable semaphore limiting the number of 
> open connections — thus avoiding the file exhaustion in the first place!)

ENFILE might be caused by a different process entirely, no?

>
> On Wednesday, April 20, 2022 at 10:49:20 PM UTC-4 Ian Lance Taylor wrote:
>>
>> On Wed, Apr 20, 2022 at 6:46 PM 'Damien Neil' via golang-nuts
>>  wrote:
>> >
>> > The reason for deprecating Temporary is that the set of "temporary" errors 
>> > was extremely ill-defined. The initial issue for 
>> > https://go.dev/issue/45729 discusses the de facto definition of Temporary 
>> > and the confusion resulting from it.
>> >
>> > Perhaps there's a useful definition of temporary or retriable errors, 
>> > perhaps limited in scope to syscall errors such as EINTR and EMFILE. I 
>> > don't know what that definition is, but perhaps we should come up with one 
>> > and add an os.ErrTemporary or some such. I don't think leaving 
>> > net.Error.Temporary undeprecated was the right choice, however; the need 
>> > for a good way to identify transient system errors such as EMFILE doesn't 
>> > mean that it was a good way to do so or could ever be made into one.
>>
>> To frame issue 45729 in a different way, whether an error is temporary
>> is not a general characteristic. It depends on the context in which
>> it appears. For the Accept loop in http.Server.Serve really the only
>> plausible temporary errors are ENFILE and EMFILE. Perhaps the net
>> package needs a RetriableAcceptError function.
>>
>> Ian
>>
>>
>>
>> > On Wednesday, April 20, 2022 at 6:02:34 PM UTC-7 ces...@gmail.com wrote:
>> >>
>> >> In Go 1.18 net.Error.Temporary was deprecated (see
>> >> https://go.dev/issue/45729). However, in trying to remove it from my
>> >> code, I found one way in which Temporary is used for which there is no
>> >> obvious replacement: in a TCP server's Accept loop, when deciding
>> >> whether to wait and retry an Accept error.
>> >>
>> >> You can see an example of this in net/http.Server today:
>> >> https://github.com/golang/go/blob/ab9d31da9e088a271e656120a3d99cd3b1103ab6/src/net/http/server.go#L3047-L3059
>> >>
>> >> In this case, Temporary seems useful, and enumerating the OS-specific
>> >> errors myself doesn't seem like a good idea.
>> >>
>> >> Does anyone have a good solution here? It doesn't seem like this was
>> >> adequately considered when making this deprecation decision.
>> >>
>> >> Caleb
>> >
>> > --
>> > You received this message because you are subscribed to the Google Groups 
>> > "golang-nuts" group.
>> > To unsubscribe from this group and stop receiving emails from it, send an 
>> > email to golang-nuts...@googlegroups.com.
>> > To view this discussion on the web visit 
>> > https://groups.google.com/d/msgid/golang-nuts/1024e668-795f-454f-a659-ab5a4bf9517cn%40googlegroups.com.
>
> --
> You received this message because you are subscribed to the Google Groups 
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to golang-nuts+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/golang-nuts/1826b3b5-c147-4015-9769-984fd84eacb3n%40googlegroups.com.

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAGeFq%2B%3D3R%3D7zaXq9nHEmVFkyg6QH5mxwoerZJAFOZzDpAgetjA%40mail.gmail.com.


Re: [go-nuts] Re: Replacement for net.Error.Temporary in server Accept loops

2022-04-21 Thread Caleb Spare
On Wed, Apr 20, 2022 at 6:46 PM 'Damien Neil' via golang-nuts
 wrote:
>
> The reason for deprecating Temporary is that the set of "temporary" errors 
> was extremely ill-defined. The initial issue for https://go.dev/issue/45729 
> discusses the de facto definition of Temporary and the confusion resulting 
> from it.
>
> Perhaps there's a useful definition of temporary or retriable errors, perhaps 
> limited in scope to syscall errors such as EINTR and EMFILE. I don't know 
> what that definition is, but perhaps we should come up with one and add an 
> os.ErrTemporary or some such. I don't think leaving net.Error.Temporary 
> undeprecated was the right choice, however; the need for a good way to 
> identify transient system errors such as EMFILE doesn't mean that it was a 
> good way to do so or could ever be made into one.

Thanks for the response. I definitely appreciate and agree with the
sentiment in issue 45729. As I went through a large codebase to remove
Temporary, most usages were on the client side and didn't really make
sense. I'm glad we're getting rid of it.

While the whole suite of Temporary errors isn't really coherent, the
issue is that a small subset of Temporary is still useful for Accept
loops and it doesn't have a non-deprecated replacement. As a case in
point, I presume that http.Server is going to keep using Temporary
indefinitely.

In our code, we try to weed out any deprecated functions, so I will
need to write some replacement for this use case (maybe using Ian's
suggestion of RetriableAcceptError). Perhaps I'll do that and send a
proposal for the net package later.

But ideally I think we would've provided a replacement for this use
case before deprecating Temporary. Perhaps it is simply that this use
case wasn't identified as part of issue 45729. Near the end of the
issue you wrote

> The cases where Temporary does not imply Timeout are surprising and not 
> particularly useful.

but Accept loops seem like a counterexample to that (surprising or
not, it is certainly useful).


Caleb

>
> On Wednesday, April 20, 2022 at 6:02:34 PM UTC-7 ces...@gmail.com wrote:
>>
>> In Go 1.18 net.Error.Temporary was deprecated (see
>> https://go.dev/issue/45729). However, in trying to remove it from my
>> code, I found one way in which Temporary is used for which there is no
>> obvious replacement: in a TCP server's Accept loop, when deciding
>> whether to wait and retry an Accept error.
>>
>> You can see an example of this in net/http.Server today:
>> https://github.com/golang/go/blob/ab9d31da9e088a271e656120a3d99cd3b1103ab6/src/net/http/server.go#L3047-L3059
>>
>> In this case, Temporary seems useful, and enumerating the OS-specific
>> errors myself doesn't seem like a good idea.
>>
>> Does anyone have a good solution here? It doesn't seem like this was
>> adequately considered when making this deprecation decision.
>>
>> Caleb
>
> --
> You received this message because you are subscribed to the Google Groups 
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to golang-nuts+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/golang-nuts/1024e668-795f-454f-a659-ab5a4bf9517cn%40googlegroups.com.

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAGeFq%2BmPo%3DjfjuoE%2BVrV%2B6Y8zKY%2BJcfFVckTwjotCwnsXDHxfg%40mail.gmail.com.


Re: [go-nuts] Re: Replacement for net.Error.Temporary in server Accept loops

2022-04-21 Thread 'Bryan C. Mills' via golang-nuts
Even ENFILE and EMFILE are not necessarily blindly retriable: if the 
process has run out of files, it may be because they have leaked (for 
example, they may be reachable from deadlocked goroutines).
If that is the case, it is arguably better for the program to fail with a 
useful error than to keep retrying without making progress.

(I would argue that the retry loop in net/http.Server is a mistake, and 
should be replaced with a user-configurable semaphore limiting the number 
of open connections — thus avoiding the file exhaustion in the first place!)

On Wednesday, April 20, 2022 at 10:49:20 PM UTC-4 Ian Lance Taylor wrote:

> On Wed, Apr 20, 2022 at 6:46 PM 'Damien Neil' via golang-nuts
>  wrote:
> >
> > The reason for deprecating Temporary is that the set of "temporary" 
> errors was extremely ill-defined. The initial issue for 
> https://go.dev/issue/45729 discusses the de facto definition of Temporary 
> and the confusion resulting from it.
> >
> > Perhaps there's a useful definition of temporary or retriable errors, 
> perhaps limited in scope to syscall errors such as EINTR and EMFILE. I 
> don't know what that definition is, but perhaps we should come up with one 
> and add an os.ErrTemporary or some such. I don't think leaving 
> net.Error.Temporary undeprecated was the right choice, however; the need 
> for a good way to identify transient system errors such as EMFILE doesn't 
> mean that it was a good way to do so or could ever be made into one.
>
> To frame issue 45729 in a different way, whether an error is temporary
> is not a general characteristic. It depends on the context in which
> it appears. For the Accept loop in http.Server.Serve really the only
> plausible temporary errors are ENFILE and EMFILE. Perhaps the net
> package needs a RetriableAcceptError function.
>
> Ian
>
>
>
> > On Wednesday, April 20, 2022 at 6:02:34 PM UTC-7 ces...@gmail.com wrote:
> >>
> >> In Go 1.18 net.Error.Temporary was deprecated (see
> >> https://go.dev/issue/45729). However, in trying to remove it from my
> >> code, I found one way in which Temporary is used for which there is no
> >> obvious replacement: in a TCP server's Accept loop, when deciding
> >> whether to wait and retry an Accept error.
> >>
> >> You can see an example of this in net/http.Server today:
> >> 
> https://github.com/golang/go/blob/ab9d31da9e088a271e656120a3d99cd3b1103ab6/src/net/http/server.go#L3047-L3059
> >>
> >> In this case, Temporary seems useful, and enumerating the OS-specific
> >> errors myself doesn't seem like a good idea.
> >>
> >> Does anyone have a good solution here? It doesn't seem like this was
> >> adequately considered when making this deprecation decision.
> >>
> >> Caleb
> >
> > --
> > You received this message because you are subscribed to the Google 
> Groups "golang-nuts" group.
> > To unsubscribe from this group and stop receiving emails from it, send 
> an email to golang-nuts...@googlegroups.com.
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/golang-nuts/1024e668-795f-454f-a659-ab5a4bf9517cn%40googlegroups.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/1826b3b5-c147-4015-9769-984fd84eacb3n%40googlegroups.com.


Re: [go-nuts] Re: Replacement for net.Error.Temporary in server Accept loops

2022-04-20 Thread Ian Lance Taylor
On Wed, Apr 20, 2022 at 6:46 PM 'Damien Neil' via golang-nuts
 wrote:
>
> The reason for deprecating Temporary is that the set of "temporary" errors 
> was extremely ill-defined. The initial issue for https://go.dev/issue/45729 
> discusses the de facto definition of Temporary and the confusion resulting 
> from it.
>
> Perhaps there's a useful definition of temporary or retriable errors, perhaps 
> limited in scope to syscall errors such as EINTR and EMFILE. I don't know 
> what that definition is, but perhaps we should come up with one and add an 
> os.ErrTemporary or some such. I don't think leaving net.Error.Temporary 
> undeprecated was the right choice, however; the need for a good way to 
> identify transient system errors such as EMFILE doesn't mean that it was a 
> good way to do so or could ever be made into one.

To frame issue 45729 in a different way, whether an error is temporary
is not a general characteristic.  It depends on the context in which
it appears.  For the Accept loop in http.Server.Serve really the only
plausible temporary errors are ENFILE and EMFILE. Perhaps the net
package needs a RetriableAcceptError function.

Ian



> On Wednesday, April 20, 2022 at 6:02:34 PM UTC-7 ces...@gmail.com wrote:
>>
>> In Go 1.18 net.Error.Temporary was deprecated (see
>> https://go.dev/issue/45729). However, in trying to remove it from my
>> code, I found one way in which Temporary is used for which there is no
>> obvious replacement: in a TCP server's Accept loop, when deciding
>> whether to wait and retry an Accept error.
>>
>> You can see an example of this in net/http.Server today:
>> https://github.com/golang/go/blob/ab9d31da9e088a271e656120a3d99cd3b1103ab6/src/net/http/server.go#L3047-L3059
>>
>> In this case, Temporary seems useful, and enumerating the OS-specific
>> errors myself doesn't seem like a good idea.
>>
>> Does anyone have a good solution here? It doesn't seem like this was
>> adequately considered when making this deprecation decision.
>>
>> Caleb
>
> --
> You received this message because you are subscribed to the Google Groups 
> "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to golang-nuts+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/golang-nuts/1024e668-795f-454f-a659-ab5a4bf9517cn%40googlegroups.com.

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/CAOyqgcVKMUhNhpHud6HowL%2BXULuZ1-%2BOu5MgMe0pj8HB0gG_Cg%40mail.gmail.com.


[go-nuts] Re: Replacement for net.Error.Temporary in server Accept loops

2022-04-20 Thread 'Damien Neil' via golang-nuts
The reason for deprecating Temporary is that the set of "temporary" errors 
was extremely ill-defined. The initial issue for https://go.dev/issue/45729 
discusses the de facto definition of Temporary and the confusion resulting 
from it.

Perhaps there's a useful definition of temporary or retriable errors, 
perhaps limited in scope to syscall errors such as EINTR and EMFILE. I 
don't know what that definition is, but perhaps we should come up with one 
and add an os.ErrTemporary or some such. I don't think leaving 
net.Error.Temporary undeprecated was the right choice, however; the need 
for a good way to identify transient system errors such as EMFILE doesn't 
mean that it was a good way to do so or could ever be made into one.

On Wednesday, April 20, 2022 at 6:02:34 PM UTC-7 ces...@gmail.com wrote:

> In Go 1.18 net.Error.Temporary was deprecated (see
> https://go.dev/issue/45729). However, in trying to remove it from my
> code, I found one way in which Temporary is used for which there is no
> obvious replacement: in a TCP server's Accept loop, when deciding
> whether to wait and retry an Accept error.
>
> You can see an example of this in net/http.Server today:
>
> https://github.com/golang/go/blob/ab9d31da9e088a271e656120a3d99cd3b1103ab6/src/net/http/server.go#L3047-L3059
>
> In this case, Temporary seems useful, and enumerating the OS-specific
> errors myself doesn't seem like a good idea.
>
> Does anyone have a good solution here? It doesn't seem like this was
> adequately considered when making this deprecation decision.
>
> Caleb
>

-- 
You received this message because you are subscribed to the Google Groups 
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to golang-nuts+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/1024e668-795f-454f-a659-ab5a4bf9517cn%40googlegroups.com.