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

Dynamic Server Lists/Client Bootstrapping #1201

Merged
merged 166 commits into from
Jun 11, 2016
Merged

Dynamic Server Lists/Client Bootstrapping #1201

merged 166 commits into from
Jun 11, 2016

Conversation

sean-
Copy link
Contributor

@sean- sean- commented May 24, 2016

This PR includes both the dynamic rebalancing and bootstrap recovery. This is a WIP, tests and docs still need to be worked on.

return 16384
}

func testManager() (m *servers.Manager) {
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 a number of objects in the tests that need to be renamed.

@sean- sean- force-pushed the f-dyn-server-list branch 2 times, most recently from 0a99ea4 to 6f7379d Compare May 26, 2016 23:06
Addr string `mapstructure:"address"`

// Timeout is used by Consul HTTP Client
Timeout time.Duration `mapstructure:"timeout"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Verify that specification timeout: "5s" works as intended

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed, works with a weak types decoder.

@sean- sean- force-pushed the f-dyn-server-list branch from 1fe0a36 to 779c5d6 Compare June 1, 2016 08:58
// will begin polling Consul for a list of Nomad Servers. When Nomad
// Clients are heartbeating successfully with Nomad Servers, Nomad
// Clients do not poll Consul for a backup server list.
backupServerDeadline time.Time
Copy link
Contributor

Choose a reason for hiding this comment

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

consulPullHeartbeatDeadline

@sean- sean- changed the title WIP: Dynamic Server Lists Dynamic Server Lists/Client Bootstrapping Jun 1, 2016

services := make(map[string]struct{})
// Get the existing allocs
c.allocLock.RLock()
Copy link
Contributor

Choose a reason for hiding this comment

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

getAllocRunners returns a copy. Can you check that all accessors of c.allocs are using the c.allocLock

@sean- sean- force-pushed the f-dyn-server-list branch from 70c344d to a0902c3 Compare June 10, 2016 19:55
return mErr.ErrorOrNil()
}

for i := range dcs {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably get rid of this and just use something like: https://play.golang.org/p/BnIufB8mJu

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I explicitly did this so that each element is quoted: "foo", "bar" vs foo, bar because I wanted to make sure any spaces or non-ascii characters are quoted and escaped. But, good point, I forgot the + format verb modifier %+q:

https://play.golang.org/p/7xkB1onztD

@sean- sean- merged commit 9b6e720 into master Jun 11, 2016
@sean- sean- deleted the f-dyn-server-list branch June 11, 2016 22:58
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants