Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Conc: Better Structured Concurrency for Go

github.com

Post reply on HN
Print view View original

71–80 of 162 posts

Re: Conc: Better Structured Concurrency for Go

#71

Post by ikiris »

jasonhansel wrote: post #64
preseinger wrote: post #44

Earlier quoted context omitted.

> In practice, panics happen. I guess this is the crux of the issue. I don't think this is true, or needs to be true. It certainly hasn't been my experience. I think assuming panics are normal will take you down some paths that make it basically impossible to write reliable software. But, to each their own. > I'm accepting the risk that my application is left in an inconsistent state, Inconsistent state makes it impo…

> An account value that previously had balance = 0 may now have balance = 1000. Is this acceptable risk? Your entire web app process crashes due to a panic every time a request triggers an extremely rare edge case. A hacker discovers this and uses it to conduct a DoS attack. Is this acceptable risk?

Why the heck are you writing web apps that panic?

Re: Conc: Better Structured Concurrency for Go

#72

Post by ikiris »

erik_seaberg wrote: post #63
preseinger wrote: post #40

Earlier quoted context omitted.

> Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic. These are all really bad things which should never survive to production code. It is not difficult to detect and prevent them. > I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potenti…

x == y can panic if interface values contain incomparable fields in unexported nested structs, how would I check for that? Should we let it become a query of death and bet thousands of peers’ jobs on it never happening?

don't do that?

this kind of thing is why deepcompare exists to begin with

Re: Conc: Better Structured Concurrency for Go

#73

Post by jasonhansel »

ikiris wrote: post #71
jasonhansel wrote: post #64

Earlier quoted context omitted.

> An account value that previously had balance = 0 may now have balance = 1000. Is this acceptable risk? Your entire web app process crashes due to a panic every time a request triggers an extremely rare edge case. A hacker discovers this and uses it to conduct a DoS attack. Is this acceptable risk?

Why the heck are you writing web apps that panic?

Because people make mistakes?

Re: Conc: Better Structured Concurrency for Go

#74

Post by loeg »

jasonhansel wrote: post #60
preseinger wrote: post #29

Earlier quoted context omitted.

> That said, crashing the whole webserver because of one misbehaving request is not necessarily a good tradeoff. Conc moves panics into the spawning goroutine, which makes it possible to do things like catch panics at the top of a request and return a useful error to the caller, even if that error is just "nil pointer dereference" with a stacktrace. It's up to the user to decide what to do with propagated panics. The…

If your programming language handles very common errors by crashing the entire application, and if preventing these crashes is actively discouraged, then that suggests a flaw in the language itself. This would be fine for a low-level language like C where you need to allow SEGFAULTs, but designing it into a high-level language makes no sense.

Go panics should not be used for very common errors.

Re: Conc: Better Structured Concurrency for Go

#75

Post by tmerr »

camdencheek wrote: post #57
tmerr wrote: post #55

Ahh having a generic pool abstraction that collects results is tempting... nice work. I likely won't use this library though, since - I don't want to mess with panics. - The default concurrency GOMAXPROCS is almost never what I want. - Aggregated errors are almost never what I want. (I haven't read the implementation, but I worry about losing important error codes, or propagating something huge across an RPC boundary…

> nice work Thanks! > The default concurrency GOMAXPROCS is almost never what I want. FWIW, the default concurrency has been changed to "unlimited" since the 0.1.0 release. > Aggregated errors are almost never what I want. Out of curiousity, what do you want? There is an option to only keep the first error, and it's possible to unwrap the error to an array of errors that compose it if you just want a slice of errors.…

> FWIW, the default concurrency has been changed to "unlimited" since the 0.1.0 release.

Nice! Will that end up on Github?

> Out of curiousity, what do you want

Most often I want to return just the first error. Some reasons: (1) smaller error messages passed across RPC boundaries (2) original errors can be inspected as intended (e.g. error codes) (3) when the semantics are to cancel after the first error, the errors that come after that are just noise.

A couple other thoughts

  - I think the non-conc comparison code is especially hairy since it's using goroutine pools. There's nothing wrong with that and it's fast, just not the easiest to work with. Often goroutine overhead is negligible and I would bound concurrency in dumber ways e.g. by shoving a sync.Semaphore into whatever code I otherwise have
  - I like that errgroup has .Go() block when concurrency limit is reached. Based on a quick skim of the code I think(?) conc does that too, but it could use documentation

Re: Conc: Better Structured Concurrency for Go

#76

Post by vbezhenar »

kevmo314 wrote: post #7

The WaitGroup looks suspiciously like errgroup, which even has the .WithMaxGoroutines() functionality: https://pkg.go.dev/golang.org/x/sync/errgroup > A frequent problem with goroutines in long-running applications is handling panics. A goroutine spawned without a panic handler will crash the whole process on panic. This is usually undesirable. In go land, this seems desirable. Recoverable errors should be propagated…

I started to write Go recently and I very much prefer panics to errors. For example in web apps. Panic produces nice stack trace. Panic bubbles automatically, I can reduce amount of code significantly. Errors are awful and panics allow to write code without losing sanity. It's like good old exceptions. And if I really need it, I can catch panic and act as needed, e.g. return HTTP code or whatever.

Re: Conc: Better Structured Concurrency for Go

#77

Post by mappu »

erik_seaberg wrote: post #63
preseinger wrote: post #40

Earlier quoted context omitted.

> Index slice out of bounds? panic. Close a channel twice? Panic. Incorrect type assertion? Panic. Dereference nil pointer? Panic. These are all really bad things which should never survive to production code. It is not difficult to detect and prevent them. > I would argue that all of these examples which are the most common in my experience are “goroutine scoped” because the goroutine was aborted before they potenti…

x == y can panic if interface values contain incomparable fields in unexported nested structs, how would I check for that? Should we let it become a query of death and bet thousands of peers’ jobs on it never happening?

Is this really the case? Can you link to anything or an example on go.dev/play/ ?

I can find a mention of "cmp.Equal" having that behavior, but that's just a third-party package panic.

Re: Conc: Better Structured Concurrency for Go

#78

Post by erik_seaberg »

loeg wrote: post #74
jasonhansel wrote: post #60

Earlier quoted context omitted.

If your programming language handles very common errors by crashing the entire application, and if preventing these crashes is actively discouraged, then that suggests a flaw in the language itself. This would be fine for a low-level language like C where you need to allow SEGFAULTs, but designing it into a high-level language makes no sense.

Go panics should not be used for very common errors .

A lot of very common operations can panic: division, dereferencing a pointer, invoking an interface method, indexing/slicing an array/slice/string, asserting the type of an interface, and converting a slice to pointer to array. It’s possible to check, but I’ve never seen a tool that verifies you never use any of these without checking. You also have to check for nil channels, though they block forever (maybe consuming a goroutine) rather than panicking.

And there are some operations where you cannot check in advance whether a panic will happen: comparing interfaces (underlying values might not be fully comparable), indexing a map (could blow up during any concurrent write), sending to a channel (might be closed), and closing a channel.

Re: Conc: Better Structured Concurrency for Go

#79

Post by aetherane »

ikiris wrote: post #71
jasonhansel wrote: post #64

Earlier quoted context omitted.

> An account value that previously had balance = 0 may now have balance = 1000. Is this acceptable risk? Your entire web app process crashes due to a panic every time a request triggers an extremely rare edge case. A hacker discovers this and uses it to conduct a DoS attack. Is this acceptable risk?

Why the heck are you writing web apps that panic?

It is pretty easy to have accidental panics in Go, for instance due to a runtime assertion that unexpectedly failed

Re: Conc: Better Structured Concurrency for Go

#80

Post by aetherane »

ikiris wrote: post #72
erik_seaberg wrote: post #63

Earlier quoted context omitted.

x == y can panic if interface values contain incomparable fields in unexported nested structs, how would I check for that? Should we let it become a query of death and bet thousands of peers’ jobs on it never happening?

don't do that? this kind of thing is why deepcompare exists to begin with

It seems unrealistic to assume that everyone on a reasonably sized team knows all of the subtle edge cases to avoid and never makes mistakes
Post reply on HN