Skip to content

Conversation

@DrJosh9000
Copy link
Contributor

@DrJosh9000 DrJosh9000 commented Nov 10, 2025

Description

In the ping loop, skip waiting on the pingTicker when a job has just run. When a job has just run, the only wait will be the jitter.

Context

Prior to rearranging and introducing jitter, the first thing the ping loop did was always to ping. continue (implied or explicit) thus led to an immediate ping. Per the comment near the end of the loop (#1567) we want to skip the ticker and ping immediately, but in exchange, wait the full ping interval the next time.

After rearranging and introducing jitter, the first thing the ping loop does is wait for the ticker, then the jitter. But the pingTicker.Reset call after running a job was written with the expectation that a ping would happen immediately. In the new arrangement, it instead causes the loop to wait the full ping interval first.

Changes

  • Rename the first channel to skipTicker
  • Send a value on skipTicker when a job has just run
  • Delete the redundant DisconnectAfterJob conditional

Testing

  • Tests have run locally (with go test ./...). Buildkite employees may check this if the pipeline has run automatically.
  • Code is formatted (with go fmt ./...)

Disclosures / Credits

Good old fashioned wetware

// Do the ping immediately so we reduce the chances our agent is assigned a job
pingTicker.Reset(pingInterval)
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Image

hmm looks like this is at least load bearing to some tests currently, so we'll need to rework that little bit

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 I figured it out

@DrJosh9000 DrJosh9000 force-pushed the skip-ticker-after-job branch from 108ab1f to 7501aff Compare November 10, 2025 05:58
@catkins
Copy link
Contributor

catkins commented Nov 10, 2025

Thanks @DrJosh9000

@DrJosh9000 DrJosh9000 merged commit 202a11c into main Nov 10, 2025
1 check passed
@DrJosh9000 DrJosh9000 deleted the skip-ticker-after-job branch November 10, 2025 22:01
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.

3 participants