-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Stats: Add ChannelConfig & Return error on subscription #219
Stats: Add ChannelConfig & Return error on subscription #219
Conversation
2002c3a
to
885a4a1
Compare
885a4a1
to
fa37f82
Compare
// SubscribeRunnableChannel subscribes the channel and starts it if there is first subscriber coming. | ||
func SubscribeRunnableChannel(c Channel) (chan interface{}, error) { | ||
if len(c.Subscribers()) == 0 { | ||
if err := c.Start(); err != nil { | ||
return nil, err | ||
} | ||
} | ||
return c.Subscribe() | ||
} | ||
|
||
// UnsubscribeClosableChannel unsubcribes the channel and close it if there is no more subscriber. | ||
func UnsubscribeClosableChannel(c Channel, sub chan interface{}) error { | ||
if err := c.Unsubscribe(sub); err != nil { | ||
return err | ||
} | ||
if len(c.Subscribers()) == 0 { | ||
return c.Close() | ||
} | ||
return nil | ||
} |
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.
With Subscribe
and Unsubscribe returning error, a new pair of function combining Start/Subscribe and Close/Unsubscribe are added for features/stats.Channel
:
- SubscribeRunnableChannel subscribes the channel and starts it if there is first subscriber coming.
- UnsubscribeClosableChannel unsubcribes the channel and close it if there is no more subscriber.
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.
This PR may be merged. Since the author have modified these file recently and this is an improvement of last PR, I believe merge may be done quickly.
Add
ChannelConfig
for app/stats.ChannelA new protobuf message
ChannelConfig
is added to configure the behavior ofstats.Channel
:With this config, a
NewChannel
function is added:So a channel could now be created independently from
stats.Manager
. All related tests only relying onstats.Channel
now removes the depedency ofstats.Manager
.Subscribe
andUnsubscribe
returns error for features/stats.ChannelSince
SubscriberLimits
inChannelConfig
controls the maximum count of subscribers the Channel could accept at the same time,Channel.Subscribe()
does not necessarily succeed and may fail with this config.So, an
error
result is returned fromSubscribe()
andUnsubscribe(ch)
now: