Copilot commented on code in PR #476:
URL: https://github.com/apache/trafficserver-ci/pull/476#discussion_r4277242638
##########
jenkins/branch/rat.pipeline:
##########
@@ -45,14 +45,21 @@ pipeline {
branch = '*/' +
env.GITHUB_BRANCH
}
- timeout(time: 10, unit:
'MINUTES') {
- retry(3) {
+ int attempt = 0
+ retry(3) {
+ if (attempt > 0) {
+ // Spread
retries out so jobs that failed together do not come back together.
+ sleep(time: 30
* attempt + (int) (Math.random() * 60), unit: 'SECONDS')
+ }
+ attempt++
+ timeout(time: 5, unit:
'MINUTES') {
Review Comment:
The retry/backoff/per-attempt-timeout block is duplicated across many
pipeline files in this PR. To reduce future churn and the risk of drift between
pipelines, consider extracting this into a shared helper (e.g., a common method
in a loaded Groovy script or a Jenkins shared library step) and calling it from
each pipeline with parameters (retries, per-try timeout, base delay/jitter, and
the `checkout` args).
##########
jenkins/branch/rat.pipeline:
##########
@@ -45,14 +45,21 @@ pipeline {
branch = '*/' +
env.GITHUB_BRANCH
}
- timeout(time: 10, unit:
'MINUTES') {
- retry(3) {
+ int attempt = 0
+ retry(3) {
+ if (attempt > 0) {
+ // Spread
retries out so jobs that failed together do not come back together.
+ sleep(time: 30
* attempt + (int) (Math.random() * 60), unit: 'SECONDS')
+ }
+ attempt++
+ timeout(time: 5, unit:
'MINUTES') {
checkout([$class: 'GitSCM',
branches: [[name: branch]],
userRemoteConfigs: [[url: env.GITHUB_URL, refspec:
"+refs/heads/${env.GITHUB_BRANCH}:refs/remotes/origin/${env.GITHUB_BRANCH}"]],
//
Branch builds normally checkout the current branch tip. If a manually
//
requested SHA is older than this shallow depth, checkout should fail fast
//
instead of falling back to a large full-history fetch.
+ // Only
this branch is fetched, so a requested SHA must also be on it.
extensions: [[$class: 'CloneOption', honorRefspec: true, shallow: true, depth:
1000, noTags: true, timeout: 10]]
])
}
Review Comment:
Previously the branch checkout was bounded by a single `timeout(time: 10,
unit: 'MINUTES')` around the entire retry block; with that outer cap removed,
the stage can now run substantially longer (up to ~18+ minutes including
backoff and per-try timeouts). If maintaining an upper bound for the stage is
operationally important (e.g., queue pressure, executor consumption, or
alignment with job-level expectations), consider reintroducing an outer timeout
with a larger budget around the whole `retry(...) { ... }` while keeping the
inner per-attempt timeout.
##########
jenkins/github/ubuntu.pipeline:
##########
Review Comment:
There are now two timeouts governing the same operation: an outer Pipeline
`timeout(time: 5, unit: 'MINUTES')` and the Git plugin `CloneOption ...
timeout: 20`. Since the outer timeout will interrupt first, the `CloneOption`
timeout is effectively misleading (and suggests 20 minutes is acceptable).
Consider aligning the `CloneOption` timeout to be <= the outer timeout (or
removing it) to reduce confusion and keep the effective policy unambiguous.
(Same pattern appears in other pipelines in this PR.)
##########
jenkins/github/ubuntu.pipeline:
##########
Review Comment:
The PR description says the analogous change for the GitHub PR pipelines is
handled in a separate PR (#475), but this PR also updates multiple
`jenkins/github/*.pipeline` files. Either update the description to reflect
that GitHub PR pipelines are included here, or move these `jenkins/github/*`
changes into the referenced PR to keep scope consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
