search

mathpaquette / IQFeed.CSharpApiClient

IQFeed.CSharpApiClient is fastest and the most well-designed C# DTN IQFeed socket API connector available
MIT License
120 stars 43 forks source link

fix(lookup): adjust semaphore count to reduce initial burst of requests #112

Closed mathpaquette closed 3 years ago

mathpaquette commented 3 years ago

Basically, by setting the semaphore half the size of the RequestsPerSecond limit, this allow us to reduce the initial burst of requests. That way, IQFeed is happy and we dont have to tweak anything else.

Fixes #106 Fixes #108

mathpaquette commented 3 years ago

@NichUK @BunkerCoder could you please review this....

BunkerCoder commented 3 years ago

@mathpaquette - Sorry that I can't keep pace with you and @Nucs (you guys are awesome coding machines!). I've been looking at this (Release/Debug/net461/netcoreapp3.1). No major performance differences. Seeing overall averages bestrewn 49.8 and 50.1, and initial burst between 74-76 req/sec. The initial burst seems to violate the announced method's limits: " The new method is a straightforward "requests per second" rate limiting method. It is a bucket limit system similar to the Linux kernel's iptables hashlimit. As a result, you start with a full bucket of request credits. Each request consumes 1 credit and credits are re-allocated every few milliseconds unless you are already at max. If you send a request when you are out of requests, you will get an error stating too many requests (the error message is the same as before). This new method allows all customers to have the same limit. Also, customers can monitor the limit themselves, and know their real limit without trial and error. "

Taken literally, what we've got is quite a different implementation. I'll noodle on it a while longer, but I think that a fundamental change in approach may be in order. Is this way out-of-line with your thinking?

mathpaquette commented 3 years ago

Le> @mathpaquette - Sorry that I can't keep pace with you and @Nucs (you guys are awesome coding machines!).

I've been looking at this (Release/Debug/net461/netcoreapp3.1). No major performance differences. Seeing overall averages bestrewn 49.8 and 50.1, and initial burst between 74-76 req/sec. The initial burst seems to violate the announced method's limits: " The new method is a straightforward "requests per second" rate limiting method. It is a bucket limit system similar to the Linux kernel's iptables hashlimit. As a result, you start with a full bucket of request credits. Each request consumes 1 credit and credits are re-allocated every few milliseconds unless you are already at max. If you send a request when you are out of requests, you will get an error stating too many requests (the error message is the same as before). This new method allows all customers to have the same limit. Also, customers can monitor the limit themselves, and know their real limit without trial and error. "

Taken literally, what we've got is quite a different implementation. I'll noodle on it a while longer, but I think that a fundamental change in approach may be in order. Is this way out-of-line with your thinking?

@BunkerCoder regarding your statement, The initial burst seems to violate the announced method's limits: Yes it does but for a very small period of time. IQFeed is looking for the average and not at every second period. They tolerate such burst and I like it because this is just much simpler than any proposed solution and works fine :-)

Tested on my weekend download and was completely flawless. Please give it a shot in live. Basically you dont need to downscale to 49 or 47...

And.... just to add on top, this is very unlikely that you'll get a burst up to 75req/second if you have 15 clients or less because requests take time to complete and makes the burst very less.

mathpaquette commented 3 years ago

@BunkerCoder If you think bursts are harmful it's pretty straightforward to avoid them.

One more if statement or simply reduce the semaphore size but if IQFeed tolerate them it's almost like a feature for me and we should leverage it.

I will always promote SIMPLER, well tested solution that others can easily understand and to see its solving the problem to the root.