Hacker News new | ask | show | jobs
by mcot2 4650 days ago
I'm not sure I agree with how channels are used here. What's the point of this spaces chan? Why couldn't a simple atomic counter solve this (see sync/atomic)? Why allocate a thousand bools?

    // The booleans representing the free active connection spaces.
    spaces := make(chan bool, *maxConnections)
    // Initialize the spaces
    for i := 0; i < *maxConnections; i++ {
        spaces <- true
    }
}

Is this really how people use go???

5 comments

This is generally the Go way of limiting concurrency and usually the recommended approach. Some use empty structs in place of bools, but otherwise there doesn't seem to be a problem with the approach.
yes, using a channel's buffer for resource management like this is a common pattern. I can't say that I necessarily like this pattern, but... yes, it is common.

In this case, this particular part of the application is worth questioning, because the error condition isn't reasonable. Right now, if the proxy is full, it accepts a TCP connection from a client, and then it just ... stalls. It doesn't disconnect the client, it doesn't read from the client, it just hangs. So if a client were to actually connect to this proxy when it's full, they'd just open up a connection and wait.

Using sync/atomic package isn't common. Yes, you could do it, but it would be more common to just have a goroutine with a counter in it, and a select statement to serialize increment and decrement messages.

And then there are the actual handlers, which throw away the error information if there's an error actually writing to the connection.

Looking at this some more... Why wouldn't you just make the waiting chan buffered and eliminate any tracking of connections.
Hey, poster here.

You're right that sync/atomic could've taken care of this, I wasn't aware of that package and figured channels were the way to go in Go.

As for making the waiting chan buffered, the reason I wanted to keep track of pending connections and active connections is because I'd like to proxy from a high-power server to a low-power server such as a Raspberry Pi. I agree with you that it could have done without though.

Thanks for the tips! :-)

So perhaps I was a bit harsh. Do check out this set of slides though: http://talks.golang.org/2012/concurrency.slide

One of the last slides: http://talks.golang.org/2012/concurrency.slide#54

Thanks, that's an awesome resource!
I think in this case using atmoic.Add* is much clearer, and probably better.
sync/atomic would indeed be the way to solve this problem.