Erick Arce

Results 29 comments of Erick Arce

Please update this PR

Should we pull this into first class rate limiting lib?

Can you update this review @aminabs and remove the unnecessary comments

You didn't address 2/3 of my comments?

> > One more comment, also, I honestly am very allergic to synchronization blocks, it's significantly more expensive, atomics use CAS which is more efficient, also synchronization if you ever...

I hope you are finding this review helpful I just want to make sure we do this the most efficient/correct way possible.

I do think if you left the code mostly as it was and just try and use the atomics mostly you would be fine. You will need synchronization when you...

I don't think the current iteration is threadsafe, theres a few points here that break the threadsafety. This doesn't exactly do what I've mentioned. Do you not prefer to wrap...

Lets go back to an earlier implementation and I can review thread-safety, I think we can just cut down on the sync blocks, but unless you use an atomic object...

Small update I have tracked down what is triggering the channel closing in the Netty code. [AbstractNioByteChannel.java](https://github.com/netty/netty/blob/ed0668384b393c3502c2136e3cc412a5c8c9056e/transport/src/main/java/io/netty/channel/nio/AbstractNioByteChannel.java#L153) set to close which triggers `closeOnRead(pipeline)`. This appears like the websocket is entering...