Skip to content
This repository has been archived by the owner on Feb 1, 2021. It is now read-only.

Parallel scheduling #1261

Merged
merged 5 commits into from
Oct 9, 2015
Merged

Parallel scheduling #1261

merged 5 commits into from
Oct 9, 2015

Conversation

aluzzardi
Copy link
Contributor

Work in progress for parallel scheduling for the Swarm driver.

/cc @abronan @vieux

vieux
vieux reviewed Oct 6, 2015
View reviewed changes
cluster/swarm/cluster.go
func (p *pendingContainer) ToContainer() *cluster.Container {
return &cluster.Container{
Container: dockerclient.Container{
Names: []string{"/" + p.Name},
Copy link
Contributor

Choose a reason for hiding this comment

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

should we check if name is empty here ?

@aluzzardi aluzzardi force-pushed the parallel-scheduling branch 2 times, most recently from dabd2dd to 6b27eec Compare October 6, 2015 00:47
@aluzzardi
Copy link
Contributor Author

This happened. Investigating.

2015/10/07 02:08:49 http: panic serving 127.0.0.1:53429: runtime error: invalid memory address or nil pointer dereference
goroutine 21121 [running]:
net/http.(*conn).serve.func1(0xc82cf6bc30, 0x7f9aae2f7b30, 0xc82b21f780)
        /usr/local/go/src/net/http/server.go:1287 +0xb5
github.com/docker/swarm/cluster.Containers.Get(0xc837771c00, 0xe, 0x10, 0xc8543172c0, 0x40, 0xdbd640)
        /home/ubuntu/go/src/github.com/docker/swarm/cluster/container.go:53 +0xcd3
github.com/docker/swarm/scheduler/node.(*Node).Container(0xc834adc6e0, 0xc8543172c0, 0x40, 0x13)
        /home/ubuntu/go/src/github.com/docker/swarm/scheduler/node/node.go:47 +0x54
github.com/docker/swarm/cluster/swarm.(*Cluster).listNodes(0xc8200ae240, 0x0, 0x0, 0x0)
        /home/ubuntu/go/src/github.com/docker/swarm/cluster/swarm/cluster.go:618 +0x31b
github.com/docker/swarm/cluster/swarm.(*Cluster).createContainer(0xc8200ae240, 0xc829b0e380, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/ubuntu/go/src/github.com/docker/swarm/cluster/swarm/cluster.go:138 +0x3bf
github.com/docker/swarm/cluster/swarm.(*Cluster).CreateContainer(0xc8200ae240, 0xc829b0e380, 0x0, 0x0, 0x0, 0x0, 0x0)
        /home/ubuntu/go/src/github.com/docker/swarm/cluster/swarm/cluster.go:107 +0x6e
github.com/docker/swarm/api.postContainersCreate(0xc8200b72c0, 0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /home/ubuntu/go/src/github.com/docker/swarm/api/handlers.go:380 +0x295
github.com/docker/swarm/api.NewPrimary.func1(0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /home/ubuntu/go/src/github.com/docker/swarm/api/primary.go:122 +0x300
net/http.HandlerFunc.ServeHTTP(0xc820246460, 0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /usr/local/go/src/net/http/server.go:1422 +0x3a
github.com/gorilla/mux.(*Router).ServeHTTP(0xc8200ac140, 0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /home/ubuntu/go/src/github.com/docker/swarm/Godeps/_workspace/src/github.com/gorilla/mux/mux.go:98 +0x29e
github.com/docker/swarm/api.(*dispatcher).ServeHTTP(0xc8200ab040, 0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /home/ubuntu/go/src/github.com/docker/swarm/api/server.go:32 +0x9f
net/http.serverHandler.ServeHTTP(0xc820279860, 0x7f9aaa1c1aa8, 0xc829b36630, 0xc82c524d20)
        /usr/local/go/src/net/http/server.go:1862 +0x19e
net/http.(*conn).serve(0xc82cf6bc30)
        /usr/local/go/src/net/http/server.go:1361 +0xbee
created by net/http.(*Server).Serve
        /usr/local/go/src/net/http/server.go:1910 +0x3f6

@aluzzardi aluzzardi force-pushed the parallel-scheduling branch from aa5aec9 to b7260f1 Compare October 7, 2015 02:35
@aluzzardi
Copy link
Contributor Author

Fixed in last changes

@abronan
Copy link
Contributor

abronan commented Oct 7, 2015

I personally like the solution with the pending list 👍, this also gets rid of the deadlock with the pull issue. Even though the lock still stays at the scheduler level (but the scheduling decision is fast enough anyway).

Will test it and tear it apart to see if there is no more panics when issuing parallel run+create+remove requests. But SGTM!

@aluzzardi aluzzardi force-pushed the parallel-scheduling branch from b7260f1 to 2823444 Compare October 7, 2015 20:08
@aluzzardi
Copy link
Contributor Author

Made a few other changes - it's good for review now.

/cc @abronan @vieux @chanwit @jimmyxian

@aluzzardi aluzzardi changed the title WIP - Parallel scheduling Parallel scheduling Oct 8, 2015
@aluzzardi
Copy link
Contributor Author

PTAL

@jimmyxian
Copy link
Contributor

Aha, more clear than before. 👍
LGTM

@vieux
Copy link
Contributor

vieux commented Oct 9, 2015

LGTM, I'd like to wait for the mesos tests to get back to normal to make sure this PR doesn't break mesos if you don't mind

@abronan
Copy link
Contributor

abronan commented Oct 9, 2015

LGTM 🍸 (when CI happy)

@chanwit
Copy link
Contributor

chanwit commented Oct 9, 2015

I'm reading the commits to understand how it works. Allow me some more time.

@chanwit
Copy link
Contributor

chanwit commented Oct 9, 2015

A few things there. Also LGTM when CI pass :):):)

Maybe you'd like to remove "WIP:" out of commit a74d90ba message :-) ?

@aluzzardi aluzzardi force-pushed the parallel-scheduling branch from db29713 to 7c0539c Compare October 9, 2015 19:55
@aluzzardi
Copy link
Contributor Author

@chanwit Fixed! Thanks

@aluzzardi aluzzardi mentioned this pull request Oct 9, 2015
Merged
vieux added a commit that referenced this pull request Oct 9, 2015
@vieux
Merge pull request #1261 from aluzzardi/parallel-scheduling
267d7e6 
Parallel scheduling
@vieux vieux merged commit 267d7e6 into docker-archive:master Oct 9, 2015
@vieux vieux deleted the parallel-scheduling branch October 9, 2015 19:57
This was referenced Oct 9, 2015
Closed
Closed
Closed
@vieux vieux mentioned this pull request Oct 14, 2015
Closed
@abronan abronan mentioned this pull request Nov 18, 2015
Closed
ChristianKniep pushed a commit to ChristianKniep/swarm that referenced this pull request Jul 27, 2017
@vieux
Merge pull request docker-archive#1261 from aluzzardi/parallel-schedu…
d588e1d 
…ling

Parallel scheduling
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
area/scheduler impact/changelog kind/enhancement status/0-triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aluzzardi @abronan @jimmyxian @vieux @chanwit @GordonTheTurtle