Skip to content
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

Merged
merged 1 commit into from
Sep 23, 2020

Conversation

Vigilans
Copy link
Contributor

@Vigilans Vigilans commented Sep 23, 2020

Add ChannelConfig for app/stats.Channel

A new protobuf message ChannelConfig is added to configure the behavior of stats.Channel:

message ChannelConfig {
    int32 SubscriberLimit = 1;
    int32 BufferSize = 2;
    int32 BroadcastTimeout = 3;
}

With this config, a NewChannel function is added:

func NewChannel(config *ChannelConfig) *Channel {
	return &Channel{
		channel:           make(chan interface{}, config.BufferSize),
		subscriberLimit:   int(config.SubscriberLimit),
		channelBufferSize: int(config.BufferSize),
		broadcastTimeout:  time.Duration(config.BroadcastTimeout+1) * time.Millisecond,
	}
}

So a channel could now be created independently from stats.Manager. All related tests only relying on stats.Channel now removes the depedency of stats.Manager.

Subscribe and Unsubscribe returns error for features/stats.Channel

Since SubscriberLimits in ChannelConfig 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 from Subscribe() and Unsubscribe(ch) now:

interface Channel {
    Subscribe() (chan interface{}, error)
    Unsubscribe(chan interface{}) error
}

Comment on lines +38 to 57
// 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
}
Copy link
Contributor Author

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.

Copy link
Contributor

@xiaokangwang xiaokangwang left a 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.

@Loyalsoldier Loyalsoldier merged commit 788dd1e into v2fly:master Sep 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants