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

Jump to bottom

Allow adding new nodes on the fly #30

Closed
s111 opened this issue Sep 11, 2017 · 3 comments · Fixed by #120
Closed

Allow adding new nodes on the fly #30

s111 opened this issue Sep 11, 2017 · 3 comments · Fixed by #120

Comments

@s111
Copy link
Contributor

s111 commented Sep 11, 2017 •
edited
Loading

Is there any reasons that the list of nodes need to be specified up front? As we are always creating a "new configuration", I don't see a problem with adding nodes at runtime to a new configuration.

For starters:

  • Remove the nodeAddrs argument from NewManager.
  • Manager.NewConfiguration creates connections to non-existent nodes on the fly, adding them to the manager for re-use.
@meling
Copy link
Member

meling commented Sep 13, 2017

Issues #27, #28, #29, #30 and to some degree #17 are related. I've proposed a project related to these topics for the master students; it wasn't picked up by anyone so far, but I expect someone may pick it up for a master thesis.

@tormoder
Copy link
Contributor

The was previously some quirks related to this, but now I think it is pretty straightforward.

@meling meling mentioned this issue May 10, 2020
Closed
@meling
Copy link
Member

meling commented Feb 12, 2021

New nodes can now be created with

	n, err := gorums.NewNodeWithID("localhost:9091", 0)
	n, err := gorums.NewNode("localhost:9091")

And nodes can be added to the manager, which will also establish the connection via:

	err = mgr.AddNode(n)

After this, a new configuration can be created that uses the new nodes.

This isn't exactly what was proposed in this issue. I do think the idea to drop passing a list or map of nodes as input to the manager is a good one. We should pass nodes as input to at configuration creation instead.

meling added a commit that referenced this issue Feb 12, 2021
@meling
NodeList and NodeMap are now passed to Configuration
c44298c 
This is a major redesign of how we pass node lists and node maps
to the system. This now happens only through the creation of a
new configuration, but still using the WithNodeList and WithNodeMap
options. These can no longer be passed to the manager. The manager
still keeps the pool of all nodes in all configurations. A benefit
from this change is that the manager can no longer fail with an error.

As this is a relatively major change, and there were fair amount
of usage examples that had to be updated, this also required a bit
of associated clean up.

When this rewrite was more or less finished I realized that we could
also provide WithNodeIDs, which could have avoided many of the
tricky rewrites. I plan to add WithNodeIDs in a follow up commit.

Fixes #30.
@meling meling mentioned this issue Feb 12, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass nodes to Configuration instead of Manager relab/gorums
3 participants
@s111 @tormoder @meling