docs(job-retry): document that the retry check ignores enable_job_queued_check

[atsikham]

Refs #5421

Summary

The job retry function always checks that the job is still queued, independent of enable_job_queued_check. That setting only applies to the scale-up function. This PR documents this, so the behavior is not mistaken for an inconsistency.

The first version of this PR made the retry function follow the setting. After review, this was dropped, because the check has different cost and benefit in the retry path:

• The retry function runs for a small share of the events, so skipping the check saves little API budget.
• It runs up to 15 minutes after the original event. The job is then more likely cancelled or already picked up, so skipping the check would create more unused runners.

Changes

• docs/configuration.md: new paragraph in the "Job retry" section describing this behavior.
• No code changes.

Test plan

• Documentation only. The behavior described matches current main: job-retry.ts always calls isJobQueued.

[Brend-Smits]

Nice fix, this matches the scale-up behavior exactly like the issue asked for.

One thing I'm not 100% sold on though: is mirroring ENABLE_JOB_QUEUED_CHECK onto the retry path actually the right call, or just the consistent one?

On the main scale-up path, disabling the check is a rate-limit tradeoff, you skip an API call per event to save budget, and eat the occasional wasted runner. That makes sense for ephemeral runners (it's even the default when ephemeral is on) because a runner with no job to do just self-terminates, cheap mistake.

But the retry path is a different situation:

• It only fires for a small slice of events (capacity-limited or failed creation), so the rate-limit savings from skipping the check here are tiny compared to the main path.
• By the time a retry runs, up to 15 min (with backoff) has passed since the original event, way more likely the job's already been cancelled or picked up by another runner than it was on the first check.

So turning the flag off buys you almost nothing on this path but meaningfully raises the odds you spin up a runner for a job that's already gone. Feels like the retry lambda is exactly the place where you'd want to keep checking regardless of the flag, not inherit it.

And if that's really the tradeoff you want (skip the check everywhere, accept the occasional wasted runner), what's job retry even buying you at that point? Its whole value is "give this job another shot," but if you're not confirming the job is still worth a shot, you're just spending backoff cycles to spin up runners for jobs that may already be done. Feels like disabling the check plus running retry is a slightly contradictory combo, not just disabling the check on its own.

Not saying block on this, just wondering if it was a deliberate call to keep the two paths consistent, or if always-checking on retry (independent of the flag) makes more sense given the different cost/benefit here. Curious what you think. Also curious what other @github-aws-runners/terraform-aws-github-runner-maintainers think of this.

[atsikham]

The idea was consistency: ENABLE_JOB_QUEUED_CHECK before was only respected by scale-up, so retry checking anyway while flag is disabled feel like inconsistency, not deliberate choice. It was not meant to say retry should skip whenever scale-up do.

But your point about retry economics is correct: skip the check here save very little API budget, but raise the chance a lot we spin up a runner for job already resolved, since more time already passed and job more likely gone by the time retry happen.

So I think is better to decouple: retry should always check, no matter the flag, since "stay consistent with scale-up" it's wrong goal when the two path have different cost/benefit.

If maintainers agree, this PR have nothing left to fix — that's already what main do today. Then either close this PR and issue as not a bug, or I update it to documentation only, explaining why retry ignore the flag on purpose, so it don't come up again. Let me know which you prefer.

[Brend-Smits]

The idea was consistency: ENABLE_JOB_QUEUED_CHECK before was only respected by scale-up, so retry checking anyway while flag is disabled feel like inconsistency, not deliberate choice. It was not meant to say retry should skip whenever scale-up do.

But your point about retry economics is correct: skip the check here save very little API budget, but raise the chance a lot we spin up a runner for job already resolved, since more time already passed and job more likely gone by the time retry happen.

So I think is better to decouple: retry should always check, no matter the flag, since "stay consistent with scale-up" it's wrong goal when the two path have different cost/benefit.

If maintainers agree, this PR have nothing left to fix — that's already what main do today. Then either close this PR and issue as not a bug, or I update it to documentation only, explaining why retry ignore the flag on purpose, so it don't come up again. Let me know which you prefer.

Thanks! I appreciate it. I think a documentation update is all we need then. Let me know if you prefer to do it in this PR or a fresh one. Either is fine with me as long as description/title are updated.

[atsikham]

@Brend-Smits done in this PR: docs only, title and description updated.