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

DEVPROD-5498 Do not consider elapased communication time during group teardown #8506

Merged
merged 3 commits into from
Nov 27, 2024
Merged

DEVPROD-5498 Do not consider elapased communication time during group teardown #8506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0f095c
DEVPROD-5498 Do not consider elapased communication time during group…
hadjri Nov 22, 2024
65d3d5f
wording + test case
hadjri Nov 22, 2024
c380234
wording + test case
hadjri Nov 22, 2024
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
6 changes: 6 additions & 0 deletions model/host/host.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,12 @@ func (h *Host) Replace(ctx context.Context) error {

Copy link
Member

Choose a reason for hiding this comment

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

Can you please explain/add links for this staging test a bit more?

Tested in staging (executions 3 vs 4 for reference) and confirmed that without the change, a host that just ran a task group with a long teardown group is unable to pick up more tasks afterwards because it immediately gets marked by the idle termination job, whereas after the change, said host is able to continue picking up tasks.

I didn't see a difference in the host event logs between execution 3 and 4 for task 1.

Copy link
Contributor Author

@hadjri hadjri Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sure. For reference task1+task2 are part of a group, and task3 is a standalone task.

Execution 3 (with change):
task1+task2 complete on host i-0ba489af415acc35e, and then that same host is able to immediately pick up task 3 afterwards.

Execution 4 (without change):
task1+task2 complete on host i-0e34ab569dac24127, which gets hit with an idle timeout due to the task group's teardown group taking too long, and it decommissions. task3 then comes up on a new host.

Copy link
Member

Choose a reason for hiding this comment

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

I only see task three running on i-0ba489af415acc35e, I don't see it running task 1, 2 and 3.

Copy link
Member

Choose a reason for hiding this comment

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

(I'm guessing you meant i-059849d0ff65edfc0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I meant i-059849d0ff65edfc0, my bad

// GetElapsedCommunicationTime returns how long since this host has communicated with evergreen or vice versa
func (h *Host) GetElapsedCommunicationTime() time.Duration {
// If the host is currently tearing down a task group, it is not considered idle, and
// it is expected that it will not communicate with evergreen because the task has completed
// and will no longer be sending heartbeat requests.
if h.IsTearingDown() {
return 0
}
if h.LastCommunicationTime.After(h.CreationTime) {
return time.Since(h.LastCommunicationTime)
}
Expand Down
6 changes: 6 additions & 0 deletions model/host/host_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1791,11 +1791,17 @@ func TestHostElapsedCommTime(t *testing.T) {
Id: "hostWithOnlyCreateTime",
CreationTime: now.Add(-7 * time.Minute),
}
hostThatsRunningTeardown := Host{
Id: "hostThatsRunningTeardown",
CreationTime: now.Add(-7 * time.Minute),
TaskGroupTeardownStartTime: now.Add(-1 * time.Minute),
}

assert.InDelta(int64(10*time.Minute), int64(hostThatRanTask.GetElapsedCommunicationTime()), float64(1*time.Millisecond))
assert.InDelta(int64(1*time.Minute), int64(hostThatJustStarted.GetElapsedCommunicationTime()), float64(1*time.Millisecond))
assert.InDelta(int64(15*time.Minute), int64(hostWithNoCreateTime.GetElapsedCommunicationTime()), float64(1*time.Millisecond))
assert.InDelta(int64(7*time.Minute), int64(hostWithOnlyCreateTime.GetElapsedCommunicationTime()), float64(1*time.Millisecond))
assert.Zero(hostThatsRunningTeardown.GetElapsedCommunicationTime())
}

func TestHostUpsert(t *testing.T) {
Expand Down
Loading