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.
<http://redbus.in/>com is strictly prohibited. If you have received this
email in error please delete it and notify the sender immediately.Before
opening any email and/or attachments, please check them for viruses and
other defects.*
--
*::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.
<http://redbus.in/>com is strictly prohibited. If you have received this
email in error please delete it and notify the sender immediately.Before
opening any email and/or attachments, please check them for viruses and
other defects.*
--
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.
