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.

Sign up for GitHub

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

Set defaults and expose configuration of tchannel timeouts. #2173

Merged
merged 8 commits into from
Feb 26, 2020
Merged

Set defaults and expose configuration of tchannel timeouts. #2173

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b46c82
Set defaults and expose configuration of tchannel timeouts.
notbdu Feb 25, 2020
88aee00
Imports
notbdu Feb 25, 2020
df955f1
Fix test.
notbdu Feb 25, 2020
faa9b18
Address PR comments.
notbdu Feb 26, 2020
a4abffe
Set idle check interval and max idle time for query m3db client as well.
notbdu Feb 26, 2020
6163b58
Imports.
notbdu Feb 26, 2020
b4301cd
Caps.
notbdu Feb 26, 2020
fab5ce2
Merge branch 'master' into bdu/tchannel-timeout
notbdu Feb 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/cmd/services/m3dbnode/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ type DBConfiguration struct {
// of applying back-pressure or protecting the db nodes.
Limits Limits `yaml:"limits"`

// Tchannel exposes tchannel config options.
Tchannel TchannelConfiguration `yaml:"tchannel"`
// TChannel exposes tchannel config options.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "TChannel" in the comment

TChannel *TChannelConfiguration `yaml:"tchannel"`
}

// InitDefaultsAndValidate initializes all default values and validates the Configuration.
Expand Down Expand Up @@ -577,8 +577,8 @@ func IsSeedNode(initialCluster []environment.SeedNode, hostID string) bool {
return false
}

// TchannelConfiguration holds tchannel config options.
type TchannelConfiguration struct {
// TChannelConfiguration holds tchannel config options.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "TChannel" in the comment

type TChannelConfiguration struct {
MaxIdleTime time.Duration `yaml:"maxIdleTime"`
IdleCheckInterval time.Duration `yaml:"idleCheckInterval"`
}
4 changes: 1 addition & 3 deletions src/cmd/services/m3dbnode/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -714,9 +714,7 @@ func TestConfiguration(t *testing.T) {
maxOutstandingWriteRequests: 0
maxOutstandingReadRequests: 0
maxOutstandingRepairedBytes: 0
tchannel:
maxIdleTime: 0s
idleCheckInterval: 0s
tchannel: null
coordinator: null
`

Expand Down
2 changes: 1 addition & 1 deletion src/dbnode/client/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ var (
SetJitter(true),
)

// defaultChannelOptions are tchannel channel options defaults.
// defaultChannelOptions are default tchannel channel options.
defaultChannelOptions = &tchannel.ChannelOptions{
MaxIdleTime: 5 * time.Minute,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Probably would've opted for 1min max idle time and 1-2 minute idle check interval, just since 1min in this world is a very long time heh.

IdleCheckInterval: 5 * time.Minute,
Expand Down
6 changes: 3 additions & 3 deletions src/dbnode/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,9 +600,9 @@ func Run(runOpts RunOptions) {
// SetDatabase() once we've initialized it.
service = ttnode.NewService(nil, ttopts)
)
if cfg.Tchannel.MaxIdleTime > 0 && cfg.Tchannel.IdleCheckInterval > 0 {
tchannelOpts.MaxIdleTime = cfg.Tchannel.MaxIdleTime
tchannelOpts.IdleCheckInterval = cfg.Tchannel.IdleCheckInterval
if cfg.TChannel != nil {
tchannelOpts.MaxIdleTime = cfg.TChannel.MaxIdleTime
tchannelOpts.IdleCheckInterval = cfg.TChannel.IdleCheckInterval
Comment on lines +604 to +605
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these need to be greater than 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are checks in the tchannel code already.

func (o *ChannelOptions) validateIdleCheck() error {
        if o.IdleCheckInterval > 0 && o.MaxIdleTime <= 0 {
                return errMaxIdleTimeNotSet
        }

        return nil
}

func (is *idleSweep) start() {
        if is.started || is.idleCheckInterval <= 0 {
                return
        }

        is.ch.log.WithFields(
                LogField{"idleCheckInterval", is.idleCheckInterval},
                LogField{"maxIdleTime", is.maxIdleTime},
        ).Info("Starting idle connections poller.")

        is.started = true
        is.stopCh = make(chan struct{})
        go is.pollerLoop()
}

}
tchannelthriftNodeClose, err := ttnode.NewServer(service,
cfg.ListenAddress, contextPool, tchannelOpts).ListenAndServe()
Expand Down