Skip to content

Commit

Permalink
Review fixes:
Browse files Browse the repository at this point in the history
- update error message in setup.go
- add empty case in setup_test.go
- make variables names more consistent
- update copyright

Signed-off-by: Gleb Kogtev <gleb.kogtev@gmail.com>
  • Loading branch information
glebkin committed Aug 20, 2024
1 parent 35a5f22 commit e6c4c37
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 8 deletions.
10 changes: 6 additions & 4 deletions fanout.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -120,15 +122,15 @@ func (f *Fanout) runWorkers(ctx context.Context, req *request.Request) chan *res
sel = selector.NewWeightedRandSelector(f.clients, f.loadFactor)
}

workerChannel := make(chan Client, f.workerCount)
workerCh := make(chan Client, f.workerCount)
responseCh := make(chan *response, f.serverCount)
go func() {
defer close(workerChannel)
defer close(workerCh)
for i := 0; i < f.serverCount; i++ {
select {
case <-ctx.Done():
return
case workerChannel <- sel.Pick():
case workerCh <- sel.Pick():
}
}
}()
Expand All @@ -140,7 +142,7 @@ func (f *Fanout) runWorkers(ctx context.Context, req *request.Request) chan *res
for i := 0; i < f.workerCount; i++ {
go func() {
defer wg.Done()
for c := range workerChannel {
for c := range workerCh {
select {
case <-ctx.Done():
return
Expand Down
2 changes: 2 additions & 0 deletions fanout_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down
2 changes: 1 addition & 1 deletion internal/selector/rand.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down
15 changes: 15 additions & 0 deletions internal/selector/rand_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at:
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package selector

import (
Expand Down
2 changes: 1 addition & 1 deletion internal/selector/simple.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down
15 changes: 15 additions & 0 deletions internal/selector/simple_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,18 @@
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at:
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package selector

import (
Expand Down
4 changes: 3 additions & 1 deletion setup.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -277,7 +279,7 @@ func parseLoadFactor(f *Fanout, c *caddyfile.Dispenser) error {
return errors.New("load-factor should be more or equal 1")
}
if loadFactor > maxLoadFactor {
return errors.Errorf("load-factor more then max value: %d", maxLoadFactor)
return errors.Errorf("load-factor %d should be less than %d", loadFactor, maxLoadFactor)
}

f.loadFactor = append(f.loadFactor, loadFactor)
Expand Down
5 changes: 4 additions & 1 deletion setup_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Copyright (c) 2020 Doc.ai and/or its affiliates.
//
// Copyright (c) 2024 MWS and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand Down Expand Up @@ -56,10 +58,11 @@ func TestSetup(t *testing.T) {
{input: "fanout . 127.0.0.1 {\nexcept a:\nworker-count ten\n}", expectedErr: "unable to normalize 'a:'"},
{input: "fanout . 127.0.0.1 127.0.0.2 {\nnetwork XXX\n}", expectedErr: "unknown network protocol"},
{input: "fanout . 127.0.0.1 {\nserver-count -100\n}", expectedErr: "Wrong argument count or unexpected line ending"},
{input: "fanout . 127.0.0.1 {\nload-factor 150\n}", expectedErr: "load-factor more then max value: 100"},
{input: "fanout . 127.0.0.1 {\nload-factor 150\n}", expectedErr: "load-factor 150 should be less than 100"},
{input: "fanout . 127.0.0.1 {\nload-factor 0\n}", expectedErr: "load-factor should be more or equal 1"},
{input: "fanout . 127.0.0.1 {\nload-factor 50 100\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
{input: "fanout . 127.0.0.1 127.0.0.2 {\nload-factor 50\n}", expectedErr: "load-factor params count must be the same as the number of hosts"},
{input: "fanout . 127.0.0.1 127.0.0.2 {\nload-factor \n}", expectedErr: "Wrong argument count or unexpected line ending"},
}

for i, test := range tests {
Expand Down

0 comments on commit e6c4c37

Please sign in to comment.