Dont consume host resources for tasks getting STOPPED while waiting in waitingTasksQueue

[prateekchaudhry]

Summary

Context: Task Resource Accounting feature implements queuing (monitorQueuedTasks) for tasks to go through where each task waits to 'consume' resources in host_resource_manager. host_resource_manager keeps an account of each task which has acquired resources to progress for task creation/running.

If a ACS StopTask request arrives at Agent while the task is still in this queue, overseeTask - waiting in waitForHostResources, falls through and does not wait for the event from monitorQueuedTasks, ends up calling 'host_resource_manager.release()` during emitTaskEvent.

But if these STOPPED tasks are dequeued later (in docker_task_engine), they just write to the channel with no listeners after the host_resource_manager.consume() call in docker_task_engine, and the resources persist in host_resource_manager.

This PR fixes this by not calling host_resource_manager.consume() for tasks in monitorQueuedTasks whose desired status has changed to STOPPED. Some cautionary implementation steps have been taken to isolate working of monitorQueuedTasks as described in implementation

Implementation

• Added a monitorQueuedTasksLock for the main body of the loop in monitorQueuedTasks. This is to synchronize the processing of the topTask in monitorQueuedTasks and any parallel update to its desired status - say from ACS updates
• The main body of this processing in monitorQueuedTasks has been moved to a method tryDequeueWaitingTasks - to put into this critical section of monitorQueuedTasksLock
• monitorQueuedTasksLock is also used in handleDesiredStatusChange which updates the desired status of a managed task
• Adds an integ test TestHostResourceManagerStopTaskNotBlockWaitingTasks to test this behavior
• Made container names unique in existing test TestHostResourceManagerResourceUtilization for easier debugging

As a result of this implementation, consider the scenarios

1. StopTask arrives and is processed when the task is not yet processed by monitorQueuedTasks

• This is safe because when task is dequeued, it's desired status is read as STOPPED and queue returns on consumedHostResourceEvent without consuming resources

2. StopTask arrives while the task is being processed by monitorQueuedTasks

• Due to critical sections put in place, desired status will be updated after monitorQueuedTasks is done processing. Resources will be consumed, and released in emitTaskEvent

3. StopTask arrives and is processed any time after monitorQueuedTasks is done processing

• Desired status will be updated after monitorQueuedTasks is done processing. Resources will be consumed and released in emitTaskEvent

Related Containers Roadmap Issue

aws/containers-roadmap#325

Testing

Verified by

• Starting and stopping a task with stopTimeout of 300s. Starting another same task and stopped it. This task goes to the waiting queue and also returns in overseeTask goroutine. Later this task does not call consume and checked debug logs

level=info time=2023-06-15T07:22:18Z msg="Task desired status STOPPED while waiting for host resources, not consuming" taskARN="arn:aws:ecs:us-west-2:<account_id>task/taskAccounting/aa77bc7083d9401b8f4aac32424abed2"

• Added an integ test TestHostResourceManagerStopTaskNotBlockWaitingTasks and verified it succeeds. This test simulates ACS stopTask for a tasks stuck in waitingTasksQueue and verifies the resources are released in host_resource_manager

New tests cover the changes:
Yes

Description for the changelog

Dont consume host resources for tasks getting STOPPED while waiting

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[prateekchaudhry]

This is an unrelated change in another test case related to task resource accounting. Can make debugging easier in future if each task and container are named uniquely.

[Realmonia]

Also a general question with agent restart (sorry if this is me missing some context) - how are we storing tasks in queue to statefile (boltdb) when agent restart or recover from unexpected exit?

[prateekchaudhry]

Also a general question with agent restart (sorry if this is me missing some context) - how are we storing tasks in queue to statefile (boltdb) when agent restart or recover from unexpected exit?

We do not store tasks in queue to statefile. On restarts, reconcileHostResources method in docker_task_engine.go takes the tasks which have progressed beyond waitForHostResources state and restores the host_resource_manager state.

So a task which has not progressed beyond that state will queue up again, as on restart, each task's startTask is called again. Those tasks which are not pre-allocated during reconcileHostResources will thus end up queuing up again.

[Realmonia]

We do not store tasks in queue to statefile. On restarts, reconcileHostResources method in docker_task_engine.go takes the tasks which have progressed beyond waitForHostResources state and restores the host_resource_manager state.

So a task which has not progressed beyond that state will queue up again, as on restart, each task's startTask is called again. Those tasks which are not pre-allocated during reconcileHostResources will thus end up queuing up again.

Oh I understand now. So those tasks are kept in state like TaskCreated. SGTM.