Skip to content

Ping immediately after completing a job#1567

Merged
pda merged 1 commit into
buildkite:mainfrom
extemporalgenome:optimize-ping-timing
Mar 16, 2022
Merged

Ping immediately after completing a job#1567
pda merged 1 commit into
buildkite:mainfrom
extemporalgenome:optimize-ping-timing

Conversation

@extemporalgenome

Copy link
Copy Markdown
Contributor

This should minimize job "waiting for agent" time without
changing overall load on Buildkite servers.

Related: https://forum.buildkite.community/t/decrease-wait-time/1922

This should minimize job "waiting for agent" time without
changing overall load on Buildkite servers.

Related: https://forum.buildkite.community/t/decrease-wait-time/1922
Comment thread agent/agent_worker.go
Comment on lines +217 to +219
pingTicker.Reset(pingInterval)

continue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So the purpose of this continue is to skip the part at the end of the loop that waits for the next tick:

		select {
		case <-pingTicker.C:
			continue
		case <-a.stop:
			return nil
		}

And because we're effectively forcing a ping, pingTicker.Reset(pingInterval) sets the clock back to a full ping duration, so that the time since the last ping doesn't come out of the next iteration of the loop.

Makes sense 👍🏼

@extemporalgenome extemporalgenome Mar 10, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's the intent! Hopefully a middle ground between good responsiveness without an increase in overall/average load placed upon Buildkite.

@pda pda merged commit 0493899 into buildkite:main Mar 16, 2022
@pda

pda commented Mar 16, 2022

Copy link
Copy Markdown
Member

Thanks @extemporalgenome 👍🏼

@extemporalgenome extemporalgenome deleted the optimize-ping-timing branch March 16, 2022 18:54
@DrJosh9000 DrJosh9000 mentioned this pull request Nov 10, 2025
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants