Skip to content

allocator: Retry failed allocations immediately upon a deallocation #2235

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

Merged
aluzzardi merged 1 commit into from
Jun 13, 2017
Merged

allocator: Retry failed allocations immediately upon a deallocation #2235

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion manager/allocator/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ type networkContext struct {
// lastRetry is the last timestamp when unallocated
// tasks/services/networks were retried.
lastRetry time.Time

// somethingWasDeallocated indicates that we just deallocated at
// least one service/task/network, so we should retry failed
// allocations (in we are experiencing IP exhaustion and an IP was
// released).
somethingWasDeallocated bool
}

func (a *Allocator) doNetworkInit(ctx context.Context) (err error) {
Expand Down Expand Up @@ -305,6 +311,8 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) {
// resources.
if err := nc.nwkAllocator.Deallocate(n); err != nil {
log.G(ctx).WithError(err).Errorf("Failed during network free for network %s", n.ID)
} else {
nc.somethingWasDeallocated = true
}

delete(nc.unallocatedNetworks, n.ID)
Expand Down Expand Up @@ -371,6 +379,8 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) {

if err := nc.nwkAllocator.DeallocateService(s); err != nil {
log.G(ctx).WithError(err).Errorf("Failed deallocation during delete of service %s", s.ID)
} else {
nc.somethingWasDeallocated = true
}

// Remove it from unallocatedServices just in case
Expand All @@ -383,11 +393,12 @@ func (a *Allocator) doNetworkAlloc(ctx context.Context, ev events.Event) {
case state.EventCommit:
a.procTasksNetwork(ctx, false)

if time.Since(nc.lastRetry) > retryInterval {
if time.Since(nc.lastRetry) > retryInterval || nc.somethingWasDeallocated {
Copy link
Copy Markdown
Contributor

@abhi abhi Jun 9, 2017

Choose a reason for hiding this comment

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

will this still need a cluster event ?

Copy link
Copy Markdown
Collaborator Author

@aaronlehmann aaronlehmann Jun 9, 2017

Choose a reason for hiding this comment

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

Yes

Copy link
Copy Markdown
Contributor

@abhi abhi Jun 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

isnt that the main problem ? If there is no cluster event we will not allocate even if somethingWasDeallocated is set ?

Copy link
Copy Markdown
Collaborator Author

@aaronlehmann aaronlehmann Jun 10, 2017

Choose a reason for hiding this comment

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

somethingWasDeallocated is only set during an event. These events are always followed by EventCommit.

Copy link
Copy Markdown
Contributor

@abhi abhi Jun 12, 2017

Choose a reason for hiding this comment

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

sg . Thanks

a.procUnallocatedNetworks(ctx)
a.procUnallocatedServices(ctx)
a.procTasksNetwork(ctx, true)
nc.lastRetry = time.Now()
nc.somethingWasDeallocated = false
}

// Any left over tasks are moved to the unallocated set
Expand Down Expand Up @@ -432,6 +443,8 @@ func (a *Allocator) doNodeAlloc(ctx context.Context, ev events.Event) {
if nc.nwkAllocator.IsNodeAllocated(node) {
if err := nc.nwkAllocator.DeallocateNode(node); err != nil {
log.G(ctx).WithError(err).Errorf("Failed freeing network resources for node %s", node.ID)
} else {
nc.somethingWasDeallocated = true
}
}
return
Expand Down Expand Up @@ -522,6 +535,8 @@ func (a *Allocator) deallocateNodes(ctx context.Context) error {
if nc.nwkAllocator.IsNodeAllocated(node) {
if err := nc.nwkAllocator.DeallocateNode(node); err != nil {
log.G(ctx).WithError(err).Errorf("Failed freeing network resources for node %s", node.ID)
} else {
nc.somethingWasDeallocated = true
}
node.Attachment = nil
if err := a.store.Batch(func(batch *store.Batch) error {
Expand Down Expand Up @@ -638,12 +653,15 @@ func (a *Allocator) doTaskAlloc(ctx context.Context, ev events.Event) {
if nc.nwkAllocator.IsTaskAllocated(t) {
if err := nc.nwkAllocator.DeallocateTask(t); err != nil {
log.G(ctx).WithError(err).Errorf("Failed freeing network resources for task %s", t.ID)
} else {
nc.somethingWasDeallocated = true
}
}

// Cleanup any task references that might exist
delete(nc.pendingTasks, t.ID)
delete(nc.unallocatedTasks, t.ID)

return
}

Expand Down Expand Up @@ -778,6 +796,7 @@ func (a *Allocator) allocateService(ctx context.Context, s *api.Service) error {
if err := nc.nwkAllocator.DeallocateService(s); err != nil {
return err
}
nc.somethingWasDeallocated = true
}

if err := nc.nwkAllocator.AllocateService(s); err != nil {
Expand Down