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

Storable scheduler #211

Open
wants to merge 56 commits into
base: master
Choose a base branch
from
Open

Storable scheduler #211

wants to merge 56 commits into from

Conversation

itouri
Copy link
Contributor

@itouri itouri commented Nov 17, 2017

New openvdc scheduler storing offer of mesos slave noticed by mesos master.
When coming request from user, scheduler refer to stored offers and judging whether mesos slave satisfing request is exist.
When exist mesos slave satisfing request, accept the request. If it does not exist, scheduler informs the user that there are no slaves that satisfy the request.

Copy link
Contributor

@unakatsuo unakatsuo left a comment

Choose a reason for hiding this comment

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

model/store.go should not be a part of model class. This should be a private struct of scheduler or mesos package.

@@ -16,7 +16,7 @@ func TestResourceName(t *testing.T) {

func TestTypes(t *testing.T) {
assert := assert.New(t)
assert.Implements((*handlers.ResourceHandler)(nil), &LxcHandler{})
// assert.Implements((*handlers.InstanceResourceHandler)(nil), &LxcHandler{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A little time ago, InstanceResourceHandler was registered by each Handler. But now InstanceResourceHandler is not used by any handlers, because each Scheduler registering instanceSchedulerHandler to global variable of scheduler/schedule.go instead of InstanceResourceHandler. So each Hander need not implement InstanceResourceHandler. I deleted this comment.

@@ -14,6 +14,6 @@ func TestResourceName(t *testing.T) {

func TestTypes(t *testing.T) {
assert := assert.New(t)
assert.Implements((*handlers.ResourceHandler)(nil), &NullHandler{})
// assert.Implements((*handlers.InstanceResourceHandler)(nil), &NullHandler{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

@@ -16,7 +16,7 @@ func TestResourceName(t *testing.T) {

func TestTypes(t *testing.T) {
assert := assert.New(t)
assert.Implements((*handlers.ResourceHandler)(nil), &QemuHandler{})
// assert.Implements((*handlers.InstanceResourceHandler)(nil), &QemuHandler{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above.

func init() {
mesosOffer = &mesos.Offer{
Id: &mesos.OfferID{
Value: sP("d39c0128-4822-49a0-9fab-640fba518d53-O590"),
Copy link
Contributor

Choose a reason for hiding this comment

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

golang/protobuf provides conversion utilities.

proto.String("xxxxxx")

https://github.com/golang/protobuf/blob/master/proto/lib.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified to using proto library for convert to pointer.

}

type Schedule struct {
*sync.Mutex
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to expose Lock()/Unlock() methods. And also sync.RWMutex is better .

https://golang.org/pkg/sync/#RWMutex

m sync.RWMutex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified mutex from sync.Mutex to sync.RWMutex and turn to private variable.

schedule.storedOffers[offer.SlaveID] = offer
}

func (s *Schedule) Assign(inst *model.Instance) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing to take mutex lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added RLock/RUnlock next to schedule.storedOffers's "for range" function.

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