-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separately track connecting fronts and do not clear them on new configs #50
Changes from 19 commits
d16339d
cc002f7
41251ed
008e810
9ee0721
cfa8beb
bd8e939
2a65be3
7f93522
b2c702b
800f367
55e5821
12a4450
6b6fc13
9b577b9
fe2273f
988625b
7eddee1
83d7257
8be6775
d7ef158
b9307b7
67fc744
9a7741a
2954726
7bc4f00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
package fronted | ||
|
||
import ( | ||
"context" | ||
) | ||
|
||
type connectingFronts interface { | ||
onConnected(m Front) | ||
connectingFront(context.Context) (Front, error) | ||
size() int | ||
} | ||
|
||
type connecting struct { | ||
// Create a channel of fronts that are connecting. | ||
frontsCh chan Front | ||
} | ||
|
||
// Make sure that connectingFronts is a connectListener | ||
var _ connectingFronts = &connecting{} | ||
|
||
// newConnectingFronts creates a new ConnectingFronts struct with a channel of fronts that have | ||
// successfully connected. | ||
func newConnectingFronts(size int) *connecting { | ||
return &connecting{ | ||
// We overallocate the channel to avoid blocking. | ||
frontsCh: make(chan Front, size), | ||
} | ||
} | ||
Comment on lines
+21
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the doc is out of sync, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right -- it would be empty, but with a large capacity based on this call from fronted.go:
The idea is to just to make sure that it won't block on adding connecting fronts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "of fronts that have" kind of implies it would already contain some successful fronts by the time |
||
|
||
// AddFront adds a new front to the list of fronts. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also out of sync. |
||
func (cf *connecting) onConnected(m Front) { | ||
cf.frontsCh <- m | ||
} | ||
|
||
func (cf *connecting) connectingFront(ctx context.Context) (Front, error) { | ||
for { | ||
select { | ||
case <-ctx.Done(): | ||
return nil, ctx.Err() | ||
case m := <-cf.frontsCh: | ||
// The front may have stopped succeeding since we last checked, | ||
// so only return it if it's still succeeding. | ||
if m.isSucceeding() { | ||
// Add the front back to the channel. | ||
cf.frontsCh <- m | ||
return m, nil | ||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The above is a key section |
||
|
||
func (cf *connecting) size() int { | ||
return len(cf.frontsCh) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
package fronted | ||
|
||
import ( | ||
"testing" | ||
) | ||
|
||
func TestConnectingFrontsSize(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
setup func() *connecting | ||
expected int | ||
}{ | ||
{ | ||
name: "empty channel", | ||
setup: func() *connecting { | ||
return newConnectingFronts(10) | ||
}, | ||
expected: 0, | ||
}, | ||
{ | ||
name: "non-empty channel", | ||
setup: func() *connecting { | ||
cf := newConnectingFronts(10) | ||
cf.onConnected(&mockFront{}) | ||
return cf | ||
}, | ||
expected: 1, | ||
}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
cf := tt.setup() | ||
if got := cf.size(); got != tt.expected { | ||
t.Errorf("size() = %d, want %d", got, tt.expected) | ||
} | ||
}) | ||
} | ||
} |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to allocate more than
size
?