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

Network db stabilization #1860

Merged
merged 3 commits into from
Aug 1, 2017
Merged

Conversation

fcrisciani
Copy link

Several fixes aimed to optimize and strengthen the networkDB component

@fcrisciani fcrisciani force-pushed the network-db-stabilization branch 4 times, most recently from 61d3c66 to ece0dea Compare July 29, 2017 00:14
@@ -436,8 +436,7 @@ func (nDB *NetworkDB) deleteNetworkEntriesForNode(deletedNode string) {
nDB.Unlock()
}

func (nDB *NetworkDB) deleteNodeNetworkEntries(nid, node string) {
nDB.Lock()
func (nDB *NetworkDB) deleteNodeNetworkEntries(nid, node string, flush bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic is very complicated depending on the case that is being handled by this function.
Can you please document this function clearly on which cases are addressed by each block ?
Also, the name flush is a bit misleading. Consider renaming it ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also do we need the flush flag ?
If it is used to identify self, cant we simply check for node == nDB.config.NodeName ?

- Introduced back the Invalidate
- optimized the rebroadcast logic

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@fcrisciani fcrisciani force-pushed the network-db-stabilization branch from ece0dea to 1faa602 Compare August 1, 2017 20:47
// of the entries to be deleted.
// B) if the entry is owned by a remote node, then we can safely delete it. This
// ensures that if we join back this network as we receive the CREATE event for
// entries owned by remote nodes, we will accept them and we notify the application
Copy link
Contributor

Choose a reason for hiding this comment

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

Few typos...

Copy link
Contributor

@mavenugo mavenugo left a comment

Choose a reason for hiding this comment

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

Few typos. Otherwise LGTM.

Flavio Crisciani added 2 commits August 1, 2017 14:08
join/leave fixes:
 - when a node leaves the network will deletes all the other nodes entries but will keep track of its
   to make sure that other nodes if they are tcp syncing will be aware of them being deleted. (a node that
   did not yet receive the network leave will potentially tcp/sync)

add network reapTime, was not being set locally

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
Before when a node was failing, all the nodes would bump the lamport time of all their
entries. This means that if a node flap, there will be a storm of update of all the entries.
This commit on the base of the previous logic guarantees that only the node that joins back
will readvertise its own entries, the other nodes won't need to advertise again.

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@fcrisciani fcrisciani force-pushed the network-db-stabilization branch from 1faa602 to 26db0b9 Compare August 1, 2017 21:09
@mavenugo mavenugo merged commit e85aeed into moby:master Aug 1, 2017
@fcrisciani fcrisciani deleted the network-db-stabilization branch August 3, 2017 00:30
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.

2 participants