When a client is finished, it should be placed into a pool instead of being destroyed. Looking for a idiomatic Go solution with channels

[jcgarciaram]

Hello, interested in contributing to this project as I believe it will be very useful for a personal project I have in mind. Is implementing a client pool still one of the contributions you are looking for?

[flashmob]

Yes, it still needed and would be on the top of the things to do. Your contribution would be very welcome & small reward offered too. Let me know if you need more info.

[xeoncross]

Can you explain this? Why should a client be placed in a pool instead of closing the client?

[flashmob]

Hi David! So what is happening now is that the clients (of type Client struct) are created for each new connection and then destroyed when the connection closes. The old client would need to be cleaned up by the garbage collector. This can be improved so that instead of destroying the client, it can be placed back into a pool after the connection is closed, thus avoiding the garbage collector. Other possible improvements would be to start with some pre-created clients & resized dynamically as needed.

There are a few solutions / packages around, if anyone has a suggestion for which one to use, please share it, you can also create your own.

Some resources:
https://blog.cloudflare.com/recycling-memory-buffers-in-go/ (buffer i.e the client struct in our case)

Here's one using mutex https://github.com/go-baa/pool/blob/master/pool.go
https://gist.github.com/joeybloggs/796e2dff57e5d210cb2d - This type of solution looks very attractive because it uses channels rather than mutex . Just two functions, Borrow() to get a client out of the pool and Return() to put it back. Could be used as a starting point to add other features

[xeoncross]

Ah ok, I wasn't aware of the GC issue. I'm kind of surprised it would make a noticeable difference though. I would think Go could handle reaping objects with only a dozen or so properties just fine.

Then again, I don't know how long it is between GC runs or how many connections we're talking about here.

[jcgarciaram]

Thanks for all the resources @flashmob . I have an idea about how this can be accomplished without using external resources and using Go channels which provide thread safety inherently. I'll try to find some time in the next couple of days to test the approach.

[xeoncross]

It looks like it would be hard to beat the simple go-baa/pool implementation. If you don't want to use it as an external dependency you can just copy the couple dozen lines or so into the gogurrilla.go script.

[flashmob]

A quick update to let you know that there will be some refactoring done - this is to add support multiple servers. Re-writing the whole config system & splitting it up into separate files. There will be merge conflicts, so best halt any work until it's done.

[flashmob]

New version just pushed in! The config changed substantially, but should be much clearer & stable moving forward. Looking forward to the pool implementation, and whatever other contributions that may come.

[jcgarciaram]

That was fast! :) . I'll begin working on the pool changes this week.

[flashmob]

Hi jcgarciaram ! Just a quick update, new version has been pushed. This one fixes a DoS vulnerability and sets hard limits to how much input can be placed on the buffers. Hope this didn't interfere with any of your changes. Also, had a big "aha" moment with regards to the way Reader interfaces are used. Also welcome to review & get some tips on how things could be improved.

[flashmob]

Just something for the scrapbook / FYI.. Stumbled on an example where a buffer pool is used in the go library https://golang.org/src/mime/encodedword.go?#L424

[flashmob]

Hi @jcgarciaram - how have you been? Just wondering how are you going?

[flashmob]

Hi @jcgarciaram - while working on #18, it became more urgent to add the pool functionality in. So went ahead and developed this today and it will be committed to PR #19. Hope everything is OK on your end, assuming that you are just too busy with other stuff ? :-)

[flashmob]

resolved in PR #31