[go-nuts] Re: Load balancing with error feedback capabilities
Hi Matt,
Ok, if you meant the type embedding -- I usually prefer it for composition
purpose, so that I can pass around the composed type (or pointer) to
function calls. Did not find any such need.
I haven’t gotten far enough into network services to have much to say about
> the features although maybe I will in the future. I was curious about this:
>
> The buffered requests are forwarded in FIFO order when the service is
>> available next.
>
>
> wouldn’t you want to do these concurrently if possible?
>
I did give it a thought before implementing, but I considered this
situation - the cluster is down for x mins, and due to the fact no requests
are getting forwarded, the request buffer fills up quickly and might reach
the user defined concurrency limit. Now when the cluster is up, and there
are active requests being accepted too, whom should I give preference to?
Because I maintain a central work queue for both active and deferred
requests, I let the program flow decide the order of processing. Perhaps I
can build an approach to have deferred requests run concurrently if there
are no active requests in queue and switch back to sequential if there are.
Or it can be another user defined parameter.
Thanks,
Ankit
On Saturday, May 19, 2018 at 3:16:08 AM UTC+5:30, matthe...@gmail.com wrote:
>
> By embedding I meant this:
>
> type ServiceQProperties struct {
> ...
> sync.Mutex
> }
>
> which allows you to call p.Lock() instead of p.REMutex.Lock(). It’s just a
> personal preference that I was happy about when I learned about it.
>
> One thought here is you could make it a *sync.Mutex and not use a pointer
> in these function arguments. Maps vars are pointers so you are doing a
> double dereference when accessing RequestErrorLog. This avoids (*sqp) but
> there may be extra memory used and work done to copy the entire struct var
> as an argument. If you get into performance details I would look at pprof
> or package testing benchmarking to decide if it matters, but I try to
> choose cleanest code first.
>
> Thanks for sharing your verification approach.
>
> I haven’t gotten far enough into network services to have much to say
> about the features although maybe I will in the future. I was curious about
> this:
>
> The buffered requests are forwarded in FIFO order when the service is
>> available next.
>
>
> wouldn’t you want to do these concurrently if possible?
>
> Matt
>
> On Friday, May 18, 2018 at 11:50:56 AM UTC-5, Ankit Gupta wrote:
>>
>> Hi Matt,
>>
>> First of all, thanks so much for the review.
>>
>> I will revisit the package structure and take care of err handling in a
>> more idiomatic way. Few points -
>>
>> - The mutex is embedded in model.ServiceQProperties. Here is the
>> definition in model.balancer_properties.go -
>>
>> type ServiceQProperties struct {
>> ...
>> REMutex sync.Mutex
>> }
>>
>> This is used in two places - error logging to service map in
>> service_error.go and reading in select_service.go.
>>
>> - I validated 3 things (all tests were done on a cluster of aws t2 medium
>> linux machines) -
>>
>> a) Program should reduce probability of selection of errored nodes - this
>> was done by continously bringing 1 to n-1 services down/up and noting the
>> trend of requests sent to each node after every state change. As an
>> example, for a 3 node cluster with 1 node down for 2 mins, the probability
>> of selection of that node reduced (from 33%) by around 1-2% on each
>> subsequent hit.
>>
>> b) For performance testing, I used apache bench for issuing large number
>> of concurrent requests/issuing requests within a time frame and
>> benchmarking against nginx. There was a 10% difference on average that is
>> attributed to the fact that I preprocess (parse/store) all requests before
>> forwarding and that the code is not the most optimzed version right now.
>>
>> c) For deferred queue functionality, I specifically wanted to test
>> scenarios where both active and deferred requests are to be sent out
>> simultaneously and if user defined concurrency limits are still valid.
>>
>> Let me know if I should provide more details on a specific feature.
>>
>> Thanks,
>> Ankit
>>
>> On Friday, May 18, 2018 at 8:21:12 PM UTC+5:30, matthe...@gmail.com
>> wrote:
>>>
>>> Hi Ankit, thanks for the Apache license and for sharing here. Here’s a
>>> code review.
>>>
>>> These are my unfiltered opinions and I hope that they are useful.
>>>
>>> Without looking at any code yet, the packages might not be idiomatic.
>>> Packages should contain specific behavior that can be decoupled from the
>>> application. These concepts could be represented as files and symbols in
>>> package main.
>>>
>>> SQP_K_LISTENER_PORT might be more regularly represented as
>>> SQPKListenerPort.
>>>
>>> In config_manager_test.go you could embed model.ServiceQProperties and
>>> error in type Properties for better readability.
>>>
>>> The newlines in th
[go-nuts] Re: Load balancing with error feedback capabilities
By embedding I meant this:
type ServiceQProperties struct {
...
sync.Mutex
}
which allows you to call p.Lock() instead of p.REMutex.Lock(). It’s just a
personal preference that I was happy about when I learned about it.
One thought here is you could make it a *sync.Mutex and not use a pointer
in these function arguments. Maps vars are pointers so you are doing a
double dereference when accessing RequestErrorLog. This avoids (*sqp) but
there may be extra memory used and work done to copy the entire struct var
as an argument. If you get into performance details I would look at pprof
or package testing benchmarking to decide if it matters, but I try to
choose cleanest code first.
Thanks for sharing your verification approach.
I haven’t gotten far enough into network services to have much to say about
the features although maybe I will in the future. I was curious about this:
The buffered requests are forwarded in FIFO order when the service is
> available next.
wouldn’t you want to do these concurrently if possible?
Matt
On Friday, May 18, 2018 at 11:50:56 AM UTC-5, Ankit Gupta wrote:
>
> Hi Matt,
>
> First of all, thanks so much for the review.
>
> I will revisit the package structure and take care of err handling in a
> more idiomatic way. Few points -
>
> - The mutex is embedded in model.ServiceQProperties. Here is the
> definition in model.balancer_properties.go -
>
> type ServiceQProperties struct {
> ...
> REMutex sync.Mutex
> }
>
> This is used in two places - error logging to service map in
> service_error.go and reading in select_service.go.
>
> - I validated 3 things (all tests were done on a cluster of aws t2 medium
> linux machines) -
>
> a) Program should reduce probability of selection of errored nodes - this
> was done by continously bringing 1 to n-1 services down/up and noting the
> trend of requests sent to each node after every state change. As an
> example, for a 3 node cluster with 1 node down for 2 mins, the probability
> of selection of that node reduced (from 33%) by around 1-2% on each
> subsequent hit.
>
> b) For performance testing, I used apache bench for issuing large number
> of concurrent requests/issuing requests within a time frame and
> benchmarking against nginx. There was a 10% difference on average that is
> attributed to the fact that I preprocess (parse/store) all requests before
> forwarding and that the code is not the most optimzed version right now.
>
> c) For deferred queue functionality, I specifically wanted to test
> scenarios where both active and deferred requests are to be sent out
> simultaneously and if user defined concurrency limits are still valid.
>
> Let me know if I should provide more details on a specific feature.
>
> Thanks,
> Ankit
>
> On Friday, May 18, 2018 at 8:21:12 PM UTC+5:30, matthe...@gmail.com wrote:
>>
>> Hi Ankit, thanks for the Apache license and for sharing here. Here’s a
>> code review.
>>
>> These are my unfiltered opinions and I hope that they are useful.
>>
>> Without looking at any code yet, the packages might not be idiomatic.
>> Packages should contain specific behavior that can be decoupled from the
>> application. These concepts could be represented as files and symbols in
>> package main.
>>
>> SQP_K_LISTENER_PORT might be more regularly represented as
>> SQPKListenerPort.
>>
>> In config_manager_test.go you could embed model.ServiceQProperties and
>> error in type Properties for better readability.
>>
>> The newlines in the functions don’t help readability.
>>
>> Instead of this:
>>
>> if sqp, err := getProperties(getPropertyFilePath()); err == nil {
>> …
>> } else {
>> fmt.Fprintf(os.Stderr…
>>
>> this may be more readable:
>>
>> // or if …; err != nil {
>> sqp, err := getProperties(getPropertyFilePath())
>> if err != nil {
>> fmt.Fprintf(os.Stderr…
>> return
>> }
>> // regular functionality
>>
>> Generally “err == nil” shouldn’t be in the code.
>>
>> In getListener a similar improvement could be made to avoid the
>> unnecessary indentation:
>>
>> if sqp.SSLEnabled == false {
>> return …
>> }
>> // longer behavior
>>
>> In TestWorkAssignment the sqp could be simpler:
>>
>> sqp := model.ServiceQProperty{
>> ListenerPort: “5252”,
>> Proto:“http”,
>> …
>>
>> More if improvement in algorithm.ChooseServiceIndex:
>>
>> if retry != 0 {
>> return …
>> }
>> // longer behavior
>>
>> The else after “if sumErr == 0” is unnecessary.
>>
>> The mutex could be embedded in model.ServiceQProperties.
>>
>> How did you validate this program?
>>
>> Thanks,
>> Matt
>>
>> On Friday, May 18, 2018 at 2:34:20 AM UTC-5, Ankit Gupta wrote:
>>>
>>> Hello gophers,
>>>
>>> I recently built a small HTTP load balancer with capabilities built
>>> around error feedback - https://github.com/gptankit/serviceq
>>> Provides two primary functionalities - deferred request queue and
>>> probabilitically reducing errored nodes selection.
>>>
[go-nuts] Re: Load balancing with error feedback capabilities
Hi Matt,
First of all, thanks so much for the review.
I will revisit the package structure and take care of err handling in a
more idiomatic way. Few points -
- The mutex is embedded in model.ServiceQProperties. Here is the definition
in model.balancer_properties.go -
type ServiceQProperties struct {
...
REMutex sync.Mutex
}
This is used in two places - error logging to service map in
service_error.go and reading in select_service.go.
- I validated 3 things (all tests were done on a cluster of aws t2 medium
linux machines) -
a) Program should reduce probability of selection of errored nodes - this
was done by continously bringing 1 to n-1 services down/up and noting the
trend of requests sent to each node after every state change. As an
example, for a 3 node cluster with 1 node down for 2 mins, the probability
of selection of that node reduced (from 33%) by around 1-2% on each
subsequent hit.
b) For performance testing, I used apache bench for issuing large number of
concurrent requests/issuing requests within a time frame and benchmarking
against nginx. There was a 10% difference on average that is attributed to
the fact that I preprocess (parse/store) all requests before forwarding and
that the code is not the most optimzed version right now.
c) For deferred queue functionality, I specifically wanted to test
scenarios where both active and deferred requests are to be sent out
simultaneously and if user defined concurrency limits are still valid.
Let me know if I should provide more details on a specific feature.
Thanks,
Ankit
On Friday, May 18, 2018 at 8:21:12 PM UTC+5:30, matthe...@gmail.com wrote:
>
> Hi Ankit, thanks for the Apache license and for sharing here. Here’s a
> code review.
>
> These are my unfiltered opinions and I hope that they are useful.
>
> Without looking at any code yet, the packages might not be idiomatic.
> Packages should contain specific behavior that can be decoupled from the
> application. These concepts could be represented as files and symbols in
> package main.
>
> SQP_K_LISTENER_PORT might be more regularly represented as
> SQPKListenerPort.
>
> In config_manager_test.go you could embed model.ServiceQProperties and
> error in type Properties for better readability.
>
> The newlines in the functions don’t help readability.
>
> Instead of this:
>
> if sqp, err := getProperties(getPropertyFilePath()); err == nil {
> …
> } else {
> fmt.Fprintf(os.Stderr…
>
> this may be more readable:
>
> // or if …; err != nil {
> sqp, err := getProperties(getPropertyFilePath())
> if err != nil {
> fmt.Fprintf(os.Stderr…
> return
> }
> // regular functionality
>
> Generally “err == nil” shouldn’t be in the code.
>
> In getListener a similar improvement could be made to avoid the
> unnecessary indentation:
>
> if sqp.SSLEnabled == false {
> return …
> }
> // longer behavior
>
> In TestWorkAssignment the sqp could be simpler:
>
> sqp := model.ServiceQProperty{
> ListenerPort: “5252”,
> Proto:“http”,
> …
>
> More if improvement in algorithm.ChooseServiceIndex:
>
> if retry != 0 {
> return …
> }
> // longer behavior
>
> The else after “if sumErr == 0” is unnecessary.
>
> The mutex could be embedded in model.ServiceQProperties.
>
> How did you validate this program?
>
> Thanks,
> Matt
>
> On Friday, May 18, 2018 at 2:34:20 AM UTC-5, Ankit Gupta wrote:
>>
>> Hello gophers,
>>
>> I recently built a small HTTP load balancer with capabilities built
>> around error feedback - https://github.com/gptankit/serviceq
>> Provides two primary functionalities - deferred request queue and
>> probabilitically reducing errored nodes selection.
>>
>> I am using channel as a in-memory data structure for storing deferred
>> requests. Would love some feedback on this approach and on the project in
>> general.
>>
>>
>>
--
*::DISCLAIMER::
The contents of this e-mail and any attachments are confidential and
intended for the named recipient(s) only.E-mail transmission is not
guaranteed to be secure or error-free as information could be intercepted,
corrupted,lost, destroyed, arrive late or incomplete, or may contain
viruses in transmission. The e mail and its contents(with or without
referred errors) shall therefore not attach any liability on the originator
or redBus.com. Views or opinions, if any, presented in this email are
solely those of the author and may not necessarily reflect the views or
opinions of redBus.com. Any form of reproduction, dissemination, copying,
disclosure, modification,distribution and / or publication of this message
without the prior written consent of authorized representative of redbus.
[go-nuts] Re: Load balancing with error feedback capabilities
Hi Ankit, thanks for the Apache license and for sharing here. Here’s a code
review.
These are my unfiltered opinions and I hope that they are useful.
Without looking at any code yet, the packages might not be idiomatic.
Packages should contain specific behavior that can be decoupled from the
application. These concepts could be represented as files and symbols in
package main.
SQP_K_LISTENER_PORT might be more regularly represented as SQPKListenerPort.
In config_manager_test.go you could embed model.ServiceQProperties and
error in type Properties for better readability.
The newlines in the functions don’t help readability.
Instead of this:
if sqp, err := getProperties(getPropertyFilePath()); err == nil {
…
} else {
fmt.Fprintf(os.Stderr…
this may be more readable:
// or if …; err != nil {
sqp, err := getProperties(getPropertyFilePath())
if err != nil {
fmt.Fprintf(os.Stderr…
return
}
// regular functionality
Generally “err == nil” shouldn’t be in the code.
In getListener a similar improvement could be made to avoid the unnecessary
indentation:
if sqp.SSLEnabled == false {
return …
}
// longer behavior
In TestWorkAssignment the sqp could be simpler:
sqp := model.ServiceQProperty{
ListenerPort: “5252”,
Proto:“http”,
…
More if improvement in algorithm.ChooseServiceIndex:
if retry != 0 {
return …
}
// longer behavior
The else after “if sumErr == 0” is unnecessary.
The mutex could be embedded in model.ServiceQProperties.
How did you validate this program?
Thanks,
Matt
On Friday, May 18, 2018 at 2:34:20 AM UTC-5, Ankit Gupta wrote:
>
> Hello gophers,
>
> I recently built a small HTTP load balancer with capabilities built around
> error feedback - https://github.com/gptankit/serviceq
> Provides two primary functionalities - deferred request queue and
> probabilitically reducing errored nodes selection.
>
> I am using channel as a in-memory data structure for storing deferred
> requests. Would love some feedback on this approach and on the project in
> general.
>
>
>
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
