Skip to content

dispatcher: Always send task updates to agent when state <= ASSIGNED #1295

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
LK4D4 merged 1 commit into from
Aug 2, 2016
Merged

dispatcher: Always send task updates to agent when state <= ASSIGNED #1295

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
5 changes: 4 additions & 1 deletion manager/dispatcher/dispatcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,10 @@ func (d *Dispatcher) Tasks(r *api.TasksRequest, stream api.Dispatcher_TasksServe
modificationCnt++
case state.EventUpdateTask:
if oldTask, exists := tasksMap[v.Task.ID]; exists {
if equality.TasksEqualStable(oldTask, v.Task) {
// States ASSIGNED and below are set by the orchestrator/scheduler,
Copy link
Copy Markdown
Contributor

@stevvooe stevvooe Aug 2, 2016

Choose a reason for hiding this comment

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

If the task is not ASSIGNED, how can we send it to an agent?

Copy link
Copy Markdown
Collaborator Author

@aaronlehmann aaronlehmann Aug 2, 2016

Choose a reason for hiding this comment

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

If the task is not ASSIGNED, how can we send it to an agent?

This is a fair point. Filtering out these tasks would be a behavioral change, though. The aim of this PR is just to make sure the agent is notified when a task transitions into ASSIGNED.

Copy link
Copy Markdown
Contributor

@stevvooe stevvooe Aug 2, 2016

Choose a reason for hiding this comment

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

That is not what I mean. If a task is not assigned, we cannot send it to a node, since it doesn't have an assignment.

I mean, this should be impossible.

Copy link
Copy Markdown
Collaborator Author

@aaronlehmann aaronlehmann Aug 2, 2016

Choose a reason for hiding this comment

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

task.NodeID is set before the task reaches the ASSIGNED state for global services.

Copy link
Copy Markdown
Contributor

@stevvooe stevvooe Aug 2, 2016

Choose a reason for hiding this comment

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

task.NodeID is set before the task reaches the ASSIGNED state for global services.

So, if the NodeID is set, the task should be assigned.

We can't break these kinds of invariants, even if it means having a second field indicating that a task was created for assignment to a specific node. It makes making logical decisions hard.

// not the agent, so tasks in these states need to be sent to the
// agent even if nothing else has changed.
if equality.TasksEqualStable(oldTask, v.Task) && v.Task.Status.State > api.TaskStateAssigned {
// this update should not trigger action at agent
tasksMap[v.Task.ID] = v.Task
continue
Expand Down