Use random apiserver address when dialing #7488

Merged
merged 1 commit into from Jun 12, 2017

Conversation

Projects
None yet
3 participants
Owner

wallyworld commented Jun 12, 2017

Description of change

We want to ensure one controller in an HA setup is not swamped with connections.
So we shuffle the api addresses prior to dialling.

QA steps

Bootstrap, enable HA, and run some Juju operations.

It would be good to see some evidence that this actually does improve matters. Because all of the mongo writes have to go through the master, it might be better to always connect to the jujud that's running on the mongo master machine. Or not. I don't know.

api/apiclient.go
@@ -591,14 +592,27 @@ func dialAPI(info *Info, opts0 DialOpts) (*dialResult, error) {
if err != nil {
return nil, errors.Trace(err)
}
- dialInfo, err := dialWebsocketMulti(ctx, info.Addrs, path, opts)
+ dialInfo, err := dialWebsocketMulti(ctx, shuffleAddresses(info.Addrs), path, opts)
@axw

axw Jun 12, 2017

Member

I'm -1 on applying this logic to client (non-agent) connections. Perhaps move this logic to worker/apicaller?

@wallyworld

wallyworld Jun 12, 2017

Owner

moved to apicaller

api/apiclient.go
if err != nil {
return nil, errors.Trace(err)
}
logger.Infof("connection established to %q", dialInfo.urlStr)
return dialInfo, nil
}
+func init() {
+ rand.Seed(time.Now().UnixNano())
@axw

axw Jun 12, 2017

Member

assuming you do move the code to worker/apicaller, please create a worker-specific rand.Rand rather than modifying global state

axw approved these changes Jun 12, 2017

@@ -83,12 +93,18 @@ func connectFallback(
conn api.Connection, didFallback bool, err error,
) {
+ infoCopy := *info
+ if len(infoCopy.Addrs) > 1 {
+ src := rand.NewSource(time.Now().UnixNano())
@axw

axw Jun 12, 2017

Member

the Rand should really live longer than a single connection call, but I guess it's not really a big deal

Owner

wallyworld commented Jun 12, 2017

$$merge$$

Contributor

jujubot commented Jun 12, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

Contributor

jujubot commented Jun 12, 2017

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/11119

Owner

wallyworld commented Jun 12, 2017

$$merge$$

Contributor

jujubot commented Jun 12, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit ef0a9b5 into juju:2.2 Jun 12, 2017

1 check failed

github-check-merge-juju Ran tests against PR. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details

wallyworld added a commit to wallyworld/juju that referenced this pull request Jun 12, 2017

Revert "Merge pull request #7488 from wallyworld/random-apiserver"
This reverts commit ef0a9b5, reversing
changes made to 4a640f2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment