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

Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

github.com

Post reply on HN
Print view View original

21–30 of 30 posts

Re: Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

#23

Post by jamespwilliams »

Fun weekend project but definitely not production-ready (no tests, no error handling, concurrent requests will cause a race condition, etc.). If readers are looking for something production-ready to use, consider https://github.com/go-redis/redis_rate (which implements GCRA/leaky bucket), or https://github.com/ulule/limiter (which uses a much simpler algorithm, but has good middleware).

Re: Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

#24

Post by peter_l_downs »

blowski wrote: post #19
nikolayasdf123 wrote: post #17

1. some tests, over the wire preferably, would be nice 2. redis.go does not seem to be nessary it just changes signature of redis client constructor without much difference, might as well inline its contents 3. using fmt too much, if you don't need run time variables encoding, can do something more simpler. like writing to w.Write([]byte) directly. fmt uses reflect and runtime type detection, better avoid if not need…

I really like the idea of getting code reviews from Hacker News for personal projects. There's so much knowledge and passion on here, it could be really useful. It would also help for me, as a bystander, to understand the context of these recommendations. I've done a bit of Go, and some of these sound useful to know.

Selfishly, I'd love a documentation review if you (or anyone else) has the time to take a try out some golang projects I've been working on.

The code is frankly not very polished and not worth reviewing, but I'm curious to hear feedback on the documentation / README and whether or not you clearly understand what these libraries are for and how to use them.

* https://github.com/peterldowns/pgtestdb

* https://github.com/peterldowns/testy

* https://github.com/peterldowns/pgmigrate

Re: Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

#26

Post by b1-88er »

maerF0x0 wrote: post #10
b1-88er wrote: post #6

Given most of the backends use round robin for loadbalancing, having in-memory counter should be enough. Removing redis as downstream dependency is a big win. For the redis implementation, there should be fallback to in-memory counting instead blocking altogether. Currently the redis is a SPOF for the entire service.

if you're round robining clients w/o sticky assignment then you're going to get nodes*limit consumption. Not correct. Also if you give limit/nodes per node and random assign a connection, you get correct answers on average, but a really janky pattern at the edge case (a user gets a 429, and retries and succeeds, then gets 429 again as they consume those last few requests).

> if you're round robining clients w/o sticky assignment then you're going to get nodes*limit consumption. Not correct.

Fair point, using in-mem storage changes the meaning of the limit, since accounting changes to local. Something to consider in the library API.

Re: Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

#28

Post by blowski »

peter_l_downs wrote: post #24
blowski wrote: post #19

Earlier quoted context omitted.

I really like the idea of getting code reviews from Hacker News for personal projects. There's so much knowledge and passion on here, it could be really useful. It would also help for me, as a bystander, to understand the context of these recommendations. I've done a bit of Go, and some of these sound useful to know.

Selfishly, I'd love a documentation review if you (or anyone else) has the time to take a try out some golang projects I've been working on. The code is frankly not very polished and not worth reviewing, but I'm curious to hear feedback on the documentation / README and whether or not you clearly understand what these libraries are for and how to use them. * https://github.com/peterldowns/pgtestdb * https://github.co…

If that's not polished code, then I'm really curious what you consider to be polished!

* It follows Go conventions, has helpful comments and readme file.

* Given it's not user facing prod code, I'm assuming high-performance isn't critical, and the nature of what it's doing means using `once.*` is worth the trade-off with concurrency.

* It also makes sense not to actual test against a database, given that's the whole point of this package.

* The code is well-structured, with "internal" code separated.

I don't think I'm good enough at Go to be able to provide any more useful feedback than that.*

Re: Show HN: Goralim - a rate limiting pkg for Go to handle distributed workloads

#30

Post by peter_l_downs »

blowski wrote: post #28
peter_l_downs wrote: post #24

Earlier quoted context omitted.

Selfishly, I'd love a documentation review if you (or anyone else) has the time to take a try out some golang projects I've been working on. The code is frankly not very polished and not worth reviewing, but I'm curious to hear feedback on the documentation / README and whether or not you clearly understand what these libraries are for and how to use them. * https://github.com/peterldowns/pgtestdb * https://github.co…

If that's not polished code, then I'm really curious what you consider to be polished! * It follows Go conventions, has helpful comments and readme file. * Given it's not user facing prod code, I'm assuming high-performance isn't critical, and the nature of what it's doing means using `once.*` is worth the trade-off with concurrency. * It also makes sense not to actual test against a database, given that's the whole…

Thank you! Just saw this — kind of you to take the time, and I appreciate the compliments.

> Given it's not user facing prod code, I'm assuming high-performance isn't critical, and the nature of what it's doing means using `once.*` is worth the trade-off with concurrency.

I'm impressed you noticed this. I don't do a good job of explaining this in code comments or docstrings, but the linearization / contention is necessary for correctness. Basically, each time someone asks for a new testdb, the code needs to make sure the relevant user exists, and the relevant template exists, and has the migrations run on it. If these things don't exist, the test will need to create them. With many tests running in parallel, they need some way to contend and make sure that the user/template is only created once.

Because golang runs the tests of separate packages as totally separate processes, the code does the contention with "advisory locks" inside the Postgres server. When two tests are contending on these advisory locks, they have to hold a connection open to the server. As a result, server speed and max simultaneous connections are the primary limiters of how many tests can be operating in parallel and how fast your test suite runs.

I added the `once.*` helpers to move the contention "left" where possible, to the memory of the packages that are being tested. Within a package, tests can (and should) also run in parallel. The `once.*` helpers force the different tests to contend in the shared process memory, the theory being that it's faster and that it reduces the number of server queries/connections held open just waiting around on a server-side lock.

I haven't actually tested the code with and without this, and thanks to your comment I will try to do this at some point!

Post reply on HN