circuit icon indicating copy to clipboard operation
circuit copied to clipboard

Implement waiting max concurrency

Open cep21 opened this issue 7 years ago • 8 comments

Add a feature that uses https://github.com/golang/sync/blob/master/semaphore/semaphore.go to limit concurrency. In this implementation, rather than failing fast with an error, the function would wait according to the context if was at a limit of MaxConcurrentRequests

cep21 avatar Feb 05 '18 13:02 cep21

have this issue is still on to do status or it has been finished?

ray1888 avatar May 07 '20 01:05 ray1888

It is still todo. The more I thought about it, the more worried I became that it would be dangerous. Do you have a strong need for this: curious about your use case.

cep21 avatar May 07 '20 16:05 cep21

for me, it is not strong need for me, but i am curious on this issue. i want to contribute this with a PR, but maybe not that fast, this required me to finish in spare time

ray1888 avatar May 08 '20 01:05 ray1888

For this feature, I worry the circuit code is already complex and it may be best if this was implemented as a wrapper on top of the circuit. It may look like something like this

type WaitingConcurrency struct {
  sem *semaphore.Weighted
  wrapped circuit.Circuit
}

func (w *WaitingConcurrency) Execute(ctx context.Context, runFunc func(context.Context) error, fallbackFunc func(context.Context, error) error) error {
  w.sem.Acquire(ctx)
  defer w.sem.Release(1)
  return w.wrapped.Execute(ctx, runFunc, fallbackFUnc)
}

The exact code will probably be a bit different and maybe require some good error checking, but the idea is to move this code outside of this already complex package into a wrapper.

If you want to contribute something like this on your personal github, I would gladly accept a pull request to reference your implementation inside this repositories README file..

Let me know if you need more details.

cep21 avatar May 08 '20 17:05 cep21

so what i should do in this implementation? iam still not that much understand the demand of this issue, what i am understanding accroding to u

 In this implementation, rather than failing fast with an error, the function would wait according to the context if was at a limit of MaxConcurrentRequests

is that should i implement a wrapper and handling the concurrency without affecting the code of exist circuit?

ray1888 avatar May 09 '20 05:05 ray1888

so far i dive deep into this code, is just burtually return error when inside circuit.run() allowNewRun just return false

ray1888 avatar May 09 '20 06:05 ray1888

return error when inside circuit.run() allowNewRun

Rather than modify any code inside this repository, create a new struct that uses a circuit as a member.

cep21 avatar May 12 '20 16:05 cep21

I think there is no need to use semaphores for that. When we need max concurrency set it is better to fail fast and create back pressure, rather than have bunch of stalled requests.

amanbolat avatar Jun 13 '21 05:06 amanbolat