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.

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

Lower task cleanup duration #3088

Merged
merged 1 commit into from
Nov 15, 2021
Merged

Lower task cleanup duration #3088

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion agent/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const (

// minimumTaskCleanupWaitDuration specifies the minimum duration to wait before cleaning up
// a task's container. This is used to enforce sane values for the config.TaskCleanupWaitDuration field.
minimumTaskCleanupWaitDuration = 1 * time.Minute
minimumTaskCleanupWaitDuration = time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM, curious though why exactly this is in place, and are there any possible issues from not waiting long enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it was an entirely arbitrary decision. I tested various scenarios and 1s seems to work fine.
At any rate, this is just the minimum allowed value and it will only be used if customer deems it useful.


// minimumImagePullInactivityTimeout specifies the minimum amount of time for that an image can be
// 'stuck' in the pull / unpack step. Very small values are unsafe and lead to high failure rate.
Expand Down
2 changes: 1 addition & 1 deletion agent/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ func TestValidFormatParseEnvVariableDuration(t *testing.T) {

func TestInvalidTaskCleanupTimeoutOverridesToThreeHours(t *testing.T) {
defer setTestRegion()()
setTestEnv("ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION", "1s")
setTestEnv("ECS_ENGINE_TASK_CLEANUP_WAIT_DURATION", "1ms")
cfg, err := NewConfig(ec2.NewBlackholeEC2MetadataClient())
assert.NoError(t, err)

Expand Down