[client] Add peer conn init limit #3001
Conversation
Limit the peer connection initialization at 200 peers at the same time
|
|
||
| // Done decrements the internal WaitGroup counter and releases a semaphore slot. | ||
| func (sg *SemaphoreGroup) Done() { | ||
| sg.waitGroup.Done() |
There was a problem hiding this comment.
If we run once time the Add() and run multiple times the Done() then it will cause an exception. If we use exported functions of this struct from multiple go routines then this part is dangerous.
There was a problem hiding this comment.
It carries the exact same risk as using a plain sync.WaitGroup, as long as the usage guarantees 1:1 Add and Done usage it should be fine, no?
There was a problem hiding this comment.
I think the solution is simple. Just fully delete the sg.waitGroup. The exported Wait has been used by the unit test only, but we can do it in different way. After all Go routine exited, check the len of the channel. It is not nice way to check internal variables in unit test but the code will be simpler and less risky to use.
There was a problem hiding this comment.
@mohamed-essam I think this is Go philosophy question. The panic can force the developer to write, thread-safe code. I think returning with an error and handling it in the upper layer is a more defensive approach in a production-ready code. If this code snipped would be placed in internal package or it is not exported then the panic is okay from my point of view.
|
Describe your changes
Limit the peer connection initialization at 200 peers at the same time
Issue ticket number and link
Checklist